Airflow: Add config variable for UI page title

[joshkadis]

Description

Enable installations to change the page title of the /admin base route using a config variable. It would default to DAGs which is hard-coded here. But you'd be able to do something like this:

Use case / motivation

I have a few different Airflow installation that I manage. If I have them open in different browser tabs, the only way to tell which one I'm looking at is from the names of the DAGs or the browser's URL (IP address in my case, not so helpful).

Related Issues

I searched and couldn't find any.

[boring-cyborg[bot]]

Thanks for opening your first issue here! Be sure to follow the issue template!

[pcandoalmeida]

Hi there, I'm new to the project and was wondering if I could work on this?

[mik-laj]

@pcandoalmeida I assigned you to this ticket.:-D

[joshkadis]

Ha, I was just coming back to ask the same thing. Good luck @pcandoalmeida !

[pcandoalmeida]

Thanks @joshkadis! I've checked the linked code and I believe the referenced line renders the browser's window (page) title. If I understood correctly the suggestion was to enable a configuration option for the UI DAG default header, so here? Just thought I'd clarify to make sure I understood the scope of the issue correctly.

[joshkadis]

👍 I was probably looking at both and pasted a link to the wrong line of code. What do you think about having the config apply to both places?

[pcandoalmeida]

I think we could certainly look into it as it won't involve too much more work. A thought I have is if the UI options are already extensive, it could be a bit too much and would prefer the header option myself as I think that could be really useful over the page title. But if we think adding both would be valuable, then let's try and go for it.

[joshkadis]

As far as I can, the only configurable UI option is navbar_color.

Personally I'd find it super helpful to apply this option to the document title and the <h2> in the UI. How about a variable like:

---

site_title

Sets a title to show in the browser tab and the DAGs overview page.

Type
string
Default
DAGs
Environment Variable
AIRFLOW__WEBSERVER__SITE_TITLE

[pcandoalmeida]

This could work! I've made some changes and am currently testing the header change, but if that works, I can then look to make it work for site_title.

[pcandoalmeida]

I've got this working for the pager header and the page title. Currently, it's under admin in airflow.cfg, but this can be changed if someone thinks there's a better place for this option. The text defaults correctly to DAGs when no option is provided in the config file, but I need to re-work the code so that the page_title default appears, also. At the moment, both elements default to DAGs.

I'm not sure how the environment variable option works, so I'm going to look into this next and will then look at fixing the page_title default, before finishing by writing some tests (unless not required).

[joshkadis]

Looks great to me. OH! I just thought of something. HTML encoding so you can't break the page by setting the title to <div>hello or <script>alert('Give me your credit card number')</script>

[pcandoalmeida]

Good point! I was thinking along similar lines. From what I can see, the current input gets escaped so that it does not render the code; this might be something built into Flask from some reading I have done.

[joshkadis]

Make sense, thanks for checking that out

[pcandoalmeida]

I have tested with airflow.cfg and environment variables manually and the UI gets updated accordingly. I think for now Flask handles input so that code cannot be injected into dags.html. I am a bit stuck on how I can set up some unit tests for this; not sure if @mik-laj you might be able to provide some pointers here? But I'll continue looking into this this week. Hopefully once this is done, I should be ready to raise the PR.

[mik-laj]

It's a good idea to add a test that will check that the content is securely protected against XSS attacks.
For the Web UI, it is enough for us to have integration tests. You should use test client to send HTTP request and check text in response
https://github.com/apache/airflow/blob/33f0cd2657b2e77ea3477e0c93f13f1474be628e/tests/www/test_views.py#L172

[pcandoalmeida]

Thank you for the pointers @mik-laj!

[pcandoalmeida]

Hi @joshkadis @mik-laj I've added tests:

• test_page_site_title()
• test_site_title_config_when_set()
• test_site_title_config_when_not_set()

To:

• test_configuration.py
• test_views.py

I wanted to get some feedback (if that's OK) on ways to test against XSS attacks:

    def test_page_site_title_xss_prevention(self):
        xss_string = "<script>alert('Give me your credit card number')</script>"
        with conf_vars({('webserver', 'site_title'): xss_string}):
            resp = self.client.get('home', follow_redirects=True)
            escaped_xss_string = "&lt;script&gt;alert(Give me your credit card number)&lt;/script&gt;"
            self.check_content_in_response(escaped_xss_string, resp)

    def test_page_site_title_xss_prevention(self):
        xss_string = "<script>alert('Give me your credit card number')</script>"
        with conf_vars({('webserver', 'site_title'): xss_string}):
            resp = self.client.get(
                xss_string,
                follow_redirects=True,
            )
            self.assertEqual(resp.status_code, 404)
            self.assertNotIn(xss_string,
                             resp.data.decode("utf-8"))

Both tests pass and the latter is taken from another XSS test. I am hoping this checks for the correct logic. If OK, I can start raising the PR.

[joshkadis]

That looks fine to me, thanks!

[mik-laj]

lGTM.

[pcandoalmeida]

Hi @joshkadis opened up a PR for this. @mik-laj not sure if you might be able to review when convenient/availability allows?

[pcandoalmeida]

Hi @joshkadis I've hit a bit of a stumbling block. It was suggested that I replace any view which includes airflow with the configuration text. I've done a few views but I am not able to find a way to amend others (e.g, Browser > DAG Runs). I am thinking there is a root/base view which I will need to update, but not sure just yet. Sorry for the delay on this!

[bhavaniravi]

I would love to have this feature, also have an option to change the site-title in the tab itself. One of the usecase I could see is to differentiate dev, test and prod environment

[pcandoalmeida]

Hi @bhavaniravi I've got the open issue functionality working in the open PR. I am trying to work on getting the changes updated in all possible views. I think splitting page-title and site-title could be a good idea. That way, we could work on site-title changes incrementally and then merge in once all done, but offer the open issue feature as a first step (possibly). Would be keen to know thoughts.

[joshkadis]

@pcandoalmeida Sorry... Where would page-title vssite-title be rendered? (thanks!)

[pcandoalmeida]

Hi @joshkadis we could keep page-title to the DAG homepage and site-title to the various places that "Airflow" appears in a view as suggested by @mik-laj (I could be getting this the other way round!) I found a way to update some templates, but could not seem to find the base template that rendered other views (details ^ I believe) so am working on that at the moment. But it feels like it would be good to get some feedback from project members as we have some good ideas, but we could perhaps look to get some clarity on how we could proceed this issue as there are various paths we coud take: implementing your original request, updating _all_ views with the singular config or splitting the config options between site-title and page-title. If the latter, I could update the code so it updates only the site-title and I can open open a new issue to update all the templates/views for page-title. We could get multiple contributors working on this so could perhaps get it done faster!