Skip to content

Conversation

@mdtro
Copy link
Contributor

@mdtro mdtro commented Feb 27, 2024

  • Update the UserAuthTokenAuthentication middleware to support hashed token values. As tokens are used, it will store the appropriate hash value.
  • Introduce a temporary option, apitoken.save-hash-on-create. This will be used in the model logic and in pre and post backfill migration tests in the future.

@mdtro mdtro marked this pull request as ready for review February 27, 2024 23:48
@mdtro mdtro requested review from a team as code owners February 27, 2024 23:48
@mdtro mdtro requested a review from wedamija February 27, 2024 23:48
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Feb 27, 2024
@mdtro mdtro marked this pull request as draft February 27, 2024 23:51
mdtro added a commit that referenced this pull request Mar 14, 2024
In support of: #65941

Adds an index for the `hashed_token` column on `ApiTokenReplica`.
@mdtro mdtro force-pushed the mdtro/apitoken-middleware-hash branch from 086c2f4 to 24c56a4 Compare March 15, 2024 11:05
@codecov
Copy link

codecov bot commented Mar 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.32%. Comparing base (ee2cbd0) to head (3a736dc).
Report is 5 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #65941   +/-   ##
=======================================
  Coverage   84.32%   84.32%           
=======================================
  Files        5306     5306           
  Lines      237077   237093   +16     
  Branches    41000    41001    +1     
=======================================
+ Hits       199904   199922   +18     
+ Misses      36955    36953    -2     
  Partials      218      218           
Files Coverage Δ
src/sentry/api/authentication.py 90.45% <100.00%> (+0.49%) ⬆️
src/sentry/options/defaults.py 100.00% <100.00%> (ø)
src/sentry/services/hybrid_cloud/auth/model.py 94.16% <100.00%> (+0.04%) ⬆️
src/sentry/services/hybrid_cloud/auth/serial.py 100.00% <ø> (ø)
src/sentry/services/hybrid_cloud/replica/impl.py 93.61% <ø> (ø)

... and 7 files with indirect coverage changes

JonasBa pushed a commit that referenced this pull request Mar 17, 2024
In support of: #65941

Adds an index for the `hashed_token` column on `ApiTokenReplica`.
Copy link
Member

@markstory markstory left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@mdtro mdtro merged commit 3ae4a10 into master Mar 18, 2024
@mdtro mdtro deleted the mdtro/apitoken-middleware-hash branch March 18, 2024 20:13
@mdtro mdtro added the Trigger: Revert Add to a merged PR to revert it (skips CI) label Mar 18, 2024
@getsentry-bot
Copy link
Contributor

PR reverted: 92e8bd5

getsentry-bot added a commit that referenced this pull request Mar 18, 2024
mdtro added a commit that referenced this pull request Apr 2, 2024
- Update the `UserAuthTokenAuthentication` middleware to support hashed
token values. As tokens are used, it will store the appropriate hash
value.
- Use an option, `apitoken.use-and-update-hash-rate` that will apply the
code paths using the hashed values for lookups and updating the hashed
values on plaintext tokens randomly based on the configured rate.
- Introduce a temporary option, `apitoken.save-hash-on-create`. This
will be used in the model logic and in pre and post backfill migration
tests in the future.

This previously resulted in INC-684 via PR
#65941. The `hashed_token` and
`hashed_refresh_token` columns now have indexes.
@github-actions github-actions bot locked and limited conversation to collaborators Apr 3, 2024
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 Trigger: Revert Add to a merged PR to revert it (skips CI)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants