Skip to content

Conversation

rauchy
Copy link
Contributor

@rauchy rauchy commented Jun 4, 2019

What type of PR is this? (check all applicable)

  • Bug Fix

Description

Related Tickets & Documents

Fixes #3770

Mobile & Desktop Screenshots/Recordings (if there are UI changes)

@rauchy rauchy requested a review from arikfr June 4, 2019 10:04
def load_user(user_id_with_identity):
user = api_key_load_user_from_request(request)
if user:
return user
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels almost as too simple 😆

Copy link
Contributor Author

@rauchy rauchy Jun 4, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've battled flask_login for like 8 hours, trying to get it to cooperate with user_loader and request_loader until this hit me 🤦‍♂️

@arikfr
Copy link
Member

arikfr commented Jun 5, 2019

login_user doesn't have any side effects like setting a cookie or anything like this, right?

@rauchy
Copy link
Contributor Author

rauchy commented Jun 6, 2019

@arikfr I CURLed this around, and there are no Set-Cookie headers in the response when using an apikey querystring parameter, so I believe we're safe.

@arikfr arikfr merged commit 2af8b39 into master Jun 12, 2019
@arikfr arikfr deleted the favor-api-key-auth branch June 12, 2019 08:45
harveyrendell pushed a commit to pushpay/redash that referenced this pull request Nov 14, 2019
* remove legacy session identifier support

* remove redundant test

* redirect to login to support any invalid session identifiers

* be more specific with caught errors

* use authorization according to api_key (if provided) over session
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

API Key authentication should take precedence over cookies

2 participants