Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SessionNoAuthTokenAuthentication custom authenticator #53850

Merged
merged 3 commits into from
Jul 31, 2023

Conversation

oioki
Copy link
Member

@oioki oioki commented Jul 31, 2023

Legal Boilerplate

Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. and is gonna need some rights from me in order to utilize my contributions in this here PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.

@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Jul 31, 2023
@codecov
Copy link

codecov bot commented Jul 31, 2023

Codecov Report

Merging #53850 (106c607) into master (7e4b836) will increase coverage by 0.00%.
Report is 11 commits behind head on master.
The diff coverage is 100.00%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #53850   +/-   ##
=======================================
  Coverage   79.52%   79.52%           
=======================================
  Files        4963     4964    +1     
  Lines      209669   209733   +64     
  Branches    35703    35712    +9     
=======================================
+ Hits       166737   166799   +62     
- Misses      37814    37815    +1     
- Partials     5118     5119    +1     
Files Changed Coverage Δ
src/sentry/api/authentication.py 82.97% <100.00%> (+0.56%) ⬆️
src/sentry/api/endpoints/api_tokens.py 95.91% <100.00%> (ø)

... and 9 files with indirect coverage changes

@oioki oioki changed the title CookieAuthentication custom authenticator SessionNoAuthTokenAuthentication custom authenticator Jul 31, 2023
@oioki oioki marked this pull request as ready for review July 31, 2023 15:27
@oioki oioki requested review from a team as code owners July 31, 2023 15:27
Copy link
Contributor

@EricHasegawa EricHasegawa left a comment

Choose a reason for hiding this comment

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

This PR is forbidding users from passing in a bearer token to our ApiTokensEndpoint right? The previous SessionAuthentication used would just ignore passed tokens I think (feel free to correct me), so does this mean that we now strictly forbid tokens here, and, if a user has an active session and passes a token, we 403 as well?

@EricHasegawa EricHasegawa self-requested a review July 31, 2023 16:53
@oioki oioki merged commit fad12c1 into master Jul 31, 2023
@oioki oioki deleted the oioki/cookie-authenticator branch July 31, 2023 17:04
@github-actions github-actions bot locked and limited conversation to collaborators Aug 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Scope: Backend Automatically applied to PRs that change backend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants