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.

Sign up for GitHub

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

Jump to bottom

feat: forbid generating an access token without a session #1504

Merged
merged 7 commits into from
Mar 28, 2024
Merged

feat: forbid generating an access token without a session #1504

merged 7 commits into from
Mar 28, 2024

Conversation

J0
Copy link
Contributor

@J0 J0 commented Mar 28, 2024 •
edited
Loading

What kind of change does this PR introduce?

Enforces the precondition of a valid session before one can create an access token. This supports refactors around generateAccessToken and updateMFASessionAndClaims. Also allows for stronger guarantees within the function since one can always assume there is a valid session.

There were a few test changes:

  • To mirror real world use, Access Tokens should now only exist where there is a valid session. We wrap generateAccessToken into a helper generateAccessTokenAndSession to replace previous occurrences where session was set to nil.
  • We split TestUpdatePassword into cases where reauthentication is required and reauthentication is not required. We also attach a session to two of the test cases as they were previously nil

@J0 J0 marked this pull request as ready for review March 28, 2024 08:36
@J0 J0 requested a review from a team as a code owner March 28, 2024 08:36
@J0
Merge branch 'j0/forbid_access_token_issuance_without_session' of git…
d6dfb46 
…hub.com:supabase/gotrue into j0/forbid_access_token_issuance_without_session
J0
J0 commented Mar 28, 2024
View reviewed changes
internal/api/phone_test.go Outdated Show resolved Hide resolved
@coveralls
Copy link

coveralls commented Mar 28, 2024 •
edited
Loading

Pull Request Test Coverage Report for Build 8465197021

Details

  • 6 of 11 (54.55%) changed or added relevant lines in 1 file are covered.
  • 17 unchanged lines in 4 files lost coverage.
  • Overall coverage decreased (-0.07%) to 65.112%
Changes Missing Coverage Covered Lines Changed/Added Lines %
internal/api/token.go 6 11 54.55%
Files with Coverage Reduction New Missed Lines %
internal/api/token.go 1 73.86%
internal/api/logout.go 2 80.0%
internal/models/user.go 6 79.19%
internal/api/reauthenticate.go 8 41.25%
Totals Coverage Status
Change from base Build 8463291796: -0.07%
Covered Lines: 8025
Relevant Lines: 12325
💛 - Coveralls

@J0 J0 merged commit 795e93d into master Mar 28, 2024
2 checks passed
@J0 J0 deleted the j0/forbid_access_token_issuance_without_session branch March 28, 2024 09:37
@github-actions github-actions bot mentioned this pull request Mar 28, 2024
Merged
J0 added a commit that referenced this pull request Apr 1, 2024
@J0
feat: forbid generating an access token without a session (#1504)
46399db 
## What kind of change does this PR introduce?

Enforces the precondition of a valid session before one can create an
access token. This supports refactors around `generateAccessToken` and
`updateMFASessionAndClaims`. Also allows for stronger guarantees within
the function since one can always assume there is a valid session.

There were a few test changes:
- To mirror real world use, Access Tokens should now only exist where
there is a valid session. We wrap `generateAccessToken` into a helper
`generateAccessTokenAndSession` to replace previous occurrences where
session was set to nil.
- We split TestUpdatePassword into cases where reauthentication is
required and reauthentication is not required. We also attach a session
to two of the test cases as they were previously nil
kangmingtay pushed a commit that referenced this pull request Apr 3, 2024
@github-actions
chore(master): release 2.146.0 (#1499)
8002ccb 
🤖 I have created a release *beep* *boop*
---


##
[2.146.0](v2.145.0...v2.146.0)
(2024-04-03)


### Features

* add custom sms hook
([#1474](#1474))
([0f6b29a](0f6b29a))
* forbid generating an access token without a session
([#1504](#1504))
([795e93d](795e93d))


### Bug Fixes

* add cleanup statement for anonymous users
([#1497](#1497))
([cf2372a](cf2372a))
* generate signup link should not error
([#1514](#1514))
([4fc3881](4fc3881))
* move all EmailActionTypes to mailer package
([#1510](#1510))
([765db08](765db08))
* refactor mfa and aal update methods
([#1503](#1503))
([31a5854](31a5854))
* rename from CustomSMSProvider to SendSMS
([#1513](#1513))
([c0bc37b](c0bc37b))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
uxodb pushed a commit to uxodb/auth that referenced this pull request Nov 13, 2024
@J0
feat: forbid generating an access token without a session (supabase#1504
c0a0dcc 
)

## What kind of change does this PR introduce?

Enforces the precondition of a valid session before one can create an
access token. This supports refactors around `generateAccessToken` and
`updateMFASessionAndClaims`. Also allows for stronger guarantees within
the function since one can always assume there is a valid session.

There were a few test changes:
- To mirror real world use, Access Tokens should now only exist where
there is a valid session. We wrap `generateAccessToken` into a helper
`generateAccessTokenAndSession` to replace previous occurrences where
session was set to nil.
- We split TestUpdatePassword into cases where reauthentication is
required and reauthentication is not required. We also attach a session
to two of the test cases as they were previously nil
uxodb pushed a commit to uxodb/auth that referenced this pull request Nov 13, 2024
@github-actions
chore(master): release 2.146.0 (supabase#1499)
b8f31d7 
🤖 I have created a release *beep* *boop*
---


##
[2.146.0](supabase/auth@v2.145.0...v2.146.0)
(2024-04-03)


### Features

* add custom sms hook
([supabase#1474](supabase#1474))
([0f6b29a](supabase@0f6b29a))
* forbid generating an access token without a session
([supabase#1504](supabase#1504))
([795e93d](supabase@795e93d))


### Bug Fixes

* add cleanup statement for anonymous users
([supabase#1497](supabase#1497))
([cf2372a](supabase@cf2372a))
* generate signup link should not error
([supabase#1514](supabase#1514))
([4fc3881](supabase@4fc3881))
* move all EmailActionTypes to mailer package
([supabase#1510](supabase#1510))
([765db08](supabase@765db08))
* refactor mfa and aal update methods
([supabase#1503](supabase#1503))
([31a5854](supabase@31a5854))
* rename from CustomSMSProvider to SendSMS
([supabase#1513](supabase#1513))
([c0bc37b](supabase@c0bc37b))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
LashaJini pushed a commit to LashaJini/auth that referenced this pull request Nov 13, 2024
@J0 @LashaJini
feat: forbid generating an access token without a session (supabase#1504
d6a3cb5 
)

## What kind of change does this PR introduce?

Enforces the precondition of a valid session before one can create an
access token. This supports refactors around `generateAccessToken` and
`updateMFASessionAndClaims`. Also allows for stronger guarantees within
the function since one can always assume there is a valid session.

There were a few test changes:
- To mirror real world use, Access Tokens should now only exist where
there is a valid session. We wrap `generateAccessToken` into a helper
`generateAccessTokenAndSession` to replace previous occurrences where
session was set to nil.
- We split TestUpdatePassword into cases where reauthentication is
required and reauthentication is not required. We also attach a session
to two of the test cases as they were previously nil
LashaJini pushed a commit to LashaJini/auth that referenced this pull request Nov 13, 2024
@github-actions @LashaJini
chore(master): release 2.146.0 (supabase#1499)
fae14d3 
🤖 I have created a release *beep* *boop*
---


##
[2.146.0](supabase/auth@v2.145.0...v2.146.0)
(2024-04-03)


### Features

* add custom sms hook
([supabase#1474](supabase#1474))
([0f6b29a](supabase@0f6b29a))
* forbid generating an access token without a session
([supabase#1504](supabase#1504))
([795e93d](supabase@795e93d))


### Bug Fixes

* add cleanup statement for anonymous users
([supabase#1497](supabase#1497))
([cf2372a](supabase@cf2372a))
* generate signup link should not error
([supabase#1514](supabase#1514))
([4fc3881](supabase@4fc3881))
* move all EmailActionTypes to mailer package
([supabase#1510](supabase#1510))
([765db08](supabase@765db08))
* refactor mfa and aal update methods
([supabase#1503](supabase#1503))
([31a5854](supabase@31a5854))
* rename from CustomSMSProvider to SendSMS
([supabase#1513](supabase#1513))
([c0bc37b](supabase@c0bc37b))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
LashaJini pushed a commit to LashaJini/auth that referenced this pull request Nov 15, 2024
@J0 @LashaJini
feat: forbid generating an access token without a session (supabase#1504
7b1e7a2 
)

## What kind of change does this PR introduce?

Enforces the precondition of a valid session before one can create an
access token. This supports refactors around `generateAccessToken` and
`updateMFASessionAndClaims`. Also allows for stronger guarantees within
the function since one can always assume there is a valid session.

There were a few test changes:
- To mirror real world use, Access Tokens should now only exist where
there is a valid session. We wrap `generateAccessToken` into a helper
`generateAccessTokenAndSession` to replace previous occurrences where
session was set to nil.
- We split TestUpdatePassword into cases where reauthentication is
required and reauthentication is not required. We also attach a session
to two of the test cases as they were previously nil
LashaJini pushed a commit to LashaJini/auth that referenced this pull request Nov 15, 2024
@github-actions @LashaJini
chore(master): release 2.146.0 (supabase#1499)
62cc5ed 
🤖 I have created a release *beep* *boop*
---


##
[2.146.0](supabase/auth@v2.145.0...v2.146.0)
(2024-04-03)


### Features

* add custom sms hook
([supabase#1474](supabase#1474))
([0f6b29a](supabase@0f6b29a))
* forbid generating an access token without a session
([supabase#1504](supabase#1504))
([795e93d](supabase@795e93d))


### Bug Fixes

* add cleanup statement for anonymous users
([supabase#1497](supabase#1497))
([cf2372a](supabase@cf2372a))
* generate signup link should not error
([supabase#1514](supabase#1514))
([4fc3881](supabase@4fc3881))
* move all EmailActionTypes to mailer package
([supabase#1510](supabase#1510))
([765db08](supabase@765db08))
* refactor mfa and aal update methods
([supabase#1503](supabase#1503))
([31a5854](supabase@31a5854))
* rename from CustomSMSProvider to SendSMS
([supabase#1513](supabase#1513))
([c0bc37b](supabase@c0bc37b))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kangmingtay kangmingtay kangmingtay approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@J0 @coveralls @kangmingtay