Skip to content

Commit 8c8570c

Browse files
authored
add login redirects to form-action directive (#15323)
## Change Description Restores broken auth login button ## Security Assessment - This change potentially impacts the Hail Batch instance as deployed by Broad Institute in GCP ### Impact Rating - This change has a low security impact ### Impact Description Adds an extra form-action target, on login pages only, to allow redirects to third party login hosts ### Appsec Review - [ ] Required: The impact has been assessed and approved by appsec
1 parent 157cccb commit 8c8570c

3 files changed

Lines changed: 16 additions & 3 deletions

File tree

auth/auth/auth.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@
4646
setup_aiohttp_jinja2,
4747
setup_common_static_routes,
4848
web_security_headers,
49+
web_security_headers_login_page,
4950
web_security_headers_swagger,
5051
)
5152

@@ -505,7 +506,7 @@ async def create_user(request: web.Request, _) -> web.Response:
505506

506507

507508
@routes.get('/user')
508-
@web_security_headers
509+
@web_security_headers_login_page
509510
@auth.maybe_authenticated_user
510511
async def user_page(request: web.Request, userdata: Optional[UserData]) -> web.Response:
511512
context_dict = {'cloud': CLOUD, **({'next_page': request.query['next']} if 'next' in request.query else {})}

web_common/web_common/__init__.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
setup_aiohttp_jinja2,
77
setup_common_static_routes,
88
web_security_headers,
9+
web_security_headers_login_page,
910
web_security_headers_swagger,
1011
)
1112

@@ -17,5 +18,6 @@
1718
'setup_aiohttp_jinja2',
1819
'setup_common_static_routes',
1920
'web_security_headers',
21+
'web_security_headers_login_page',
2022
'web_security_headers_swagger',
2123
]

web_common/web_common/web_common.py

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,17 @@ def web_security_headers_swagger(fun):
127127
)
128128

129129

130-
def web_security_header_generator(fun, extra_script: str = '', extra_style: str = '', extra_img: str = ''):
130+
def web_security_headers_login_page(fun):
131+
# Login/signup forms redirect through auth.hail.is/login to the OAuth provider. Chrome follows
132+
# the redirect chain when enforcing form-action, so OAuth domains must be allowed.
133+
return web_security_header_generator(
134+
fun, extra_form_action='https://accounts.google.com https://login.microsoftonline.com'
135+
)
136+
137+
138+
def web_security_header_generator(
139+
fun, extra_script: str = '', extra_style: str = '', extra_img: str = '', extra_form_action: str = ''
140+
):
131141
@wraps(fun)
132142
async def wrapped(request, *args, **kwargs):
133143
response = await fun(request, *args, **kwargs)
@@ -138,7 +148,7 @@ async def wrapped(request, *args, **kwargs):
138148
script_src = f'script-src \'self\' {extra_script} cdn.jsdelivr.net cdn.plot.ly;'
139149
img_src = f'img-src \'self\' {extra_img};'
140150
frame_ancestors = 'frame-ancestors \'self\';'
141-
form_action = 'form-action \'self\';'
151+
form_action = f"form-action 'self'{' ' + extra_form_action if extra_form_action else ''};"
142152

143153
response.headers['Content-Security-Policy'] = (
144154
f'{default_src} {font_src} {style_src} {script_src} {img_src} {frame_ancestors} {form_action}'

0 commit comments

Comments
 (0)