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

fix: improve token OIDC logging #1606

Merged
merged 2 commits into from
Jun 3, 2024
Merged

fix: improve token OIDC logging #1606

merged 2 commits into from
Jun 3, 2024

Conversation

kangmingtay
Copy link
Member

What kind of change does this PR introduce?

  • Currently, when the "Unacceptable audience in id_token" error is returned, it doesn't log the audience claim from the id token, which makes it hard to debug. The audience claim from the id token is now logged as well when this error is returned.
  • Adds a basic test for the generic id token oidc getProvider() method, since we currently have 0 coverage for this file
  • The test also uncovered a possible nil pointer panic in the case of the generic OIDC provider being returned since in the generic case, the oauthConfig will be nil. Rather than returning the oauthConfig, we only need to return the skipNonceCheck property since we only check for that.

@kangmingtay kangmingtay requested a review from a team as a code owner June 3, 2024 06:46
@coveralls
Copy link

Pull Request Test Coverage Report for Build 9345543792

Details

  • 6 of 14 (42.86%) changed or added relevant lines in 1 file are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage increased (+0.2%) to 65.948%
Changes Missing Coverage Covered Lines Changed/Added Lines %
internal/api/token_oidc.go 6 14 42.86%
Files with Coverage Reduction New Missed Lines %
internal/api/token_oidc.go 1 16.76%
Totals Coverage Status
Change from base Build 9318339367: 0.2%
Covered Lines: 8502
Relevant Lines: 12892
💛 - Coveralls

J0
J0 approved these changes Jun 3, 2024
View reviewed changes
internal/api/token_oidc.go Show resolved Hide resolved
@kangmingtay kangmingtay merged commit 5262683 into master Jun 3, 2024
3 checks passed
@kangmingtay kangmingtay deleted the km/improve-token-oidc-logging branch June 3, 2024 09:43
@github-actions github-actions bot mentioned this pull request May 31, 2024
Merged
kangmingtay pushed a commit that referenced this pull request Jun 6, 2024
@github-actions
chore(master): release 2.153.0 (#1596)
dd16b95 
🤖 I have created a release *beep* *boop*
---


##
[2.153.0](v2.152.0...v2.153.0)
(2024-06-04)


### Features

* add SAML specific external URL config
([#1599](#1599))
([b352719](b352719))
* add support for verifying argon2i and argon2id passwords
([#1597](#1597))
([55409f7](55409f7))
* make the email client explicity set the format to be HTML
([#1149](#1149))
([53e223a](53e223a))


### Bug Fixes

* call write header in write if not written
([#1598](#1598))
([0ef7eb3](0ef7eb3))
* deadlock issue with timeout middleware write
([#1595](#1595))
([6c9fbd4](6c9fbd4))
* improve token OIDC logging
([#1606](#1606))
([5262683](5262683))
* update contributing to use v1.22
([#1609](#1609))
([5894d9e](5894d9e))

---
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
@kangmingtay
fix: improve token OIDC logging (supabase#1606)
d324df7 
## What kind of change does this PR introduce?
* Currently, when the "Unacceptable audience in id_token" error is
returned, it doesn't log the audience claim from the id token, which
makes it hard to debug. The audience claim from the id token is now
logged as well when this error is returned.
* Adds a basic test for the generic id token oidc `getProvider()`
method, since we currently have 0 coverage for this file
* The test also uncovered a possible nil pointer panic in the case of
the generic OIDC provider being returned since in the generic case, the
`oauthConfig` will be nil. Rather than returning the `oauthConfig`, we
only need to return the `skipNonceCheck` property since we only check
for that.
uxodb pushed a commit to uxodb/auth that referenced this pull request Nov 13, 2024
@github-actions
chore(master): release 2.153.0 (supabase#1596)
bf00d92 
🤖 I have created a release *beep* *boop*
---


##
[2.153.0](supabase/auth@v2.152.0...v2.153.0)
(2024-06-04)


### Features

* add SAML specific external URL config
([supabase#1599](supabase#1599))
([b352719](supabase@b352719))
* add support for verifying argon2i and argon2id passwords
([supabase#1597](supabase#1597))
([55409f7](supabase@55409f7))
* make the email client explicity set the format to be HTML
([supabase#1149](supabase#1149))
([53e223a](supabase@53e223a))


### Bug Fixes

* call write header in write if not written
([supabase#1598](supabase#1598))
([0ef7eb3](supabase@0ef7eb3))
* deadlock issue with timeout middleware write
([supabase#1595](supabase#1595))
([6c9fbd4](supabase@6c9fbd4))
* improve token OIDC logging
([supabase#1606](supabase#1606))
([5262683](supabase@5262683))
* update contributing to use v1.22
([supabase#1609](supabase#1609))
([5894d9e](supabase@5894d9e))

---
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
@kangmingtay @LashaJini
fix: improve token OIDC logging (supabase#1606)
6923d3a 
## What kind of change does this PR introduce?
* Currently, when the "Unacceptable audience in id_token" error is
returned, it doesn't log the audience claim from the id token, which
makes it hard to debug. The audience claim from the id token is now
logged as well when this error is returned.
* Adds a basic test for the generic id token oidc `getProvider()`
method, since we currently have 0 coverage for this file
* The test also uncovered a possible nil pointer panic in the case of
the generic OIDC provider being returned since in the generic case, the
`oauthConfig` will be nil. Rather than returning the `oauthConfig`, we
only need to return the `skipNonceCheck` property since we only check
for that.
LashaJini pushed a commit to LashaJini/auth that referenced this pull request Nov 13, 2024
@github-actions @LashaJini
chore(master): release 2.153.0 (supabase#1596)
569e3ac 
🤖 I have created a release *beep* *boop*
---


##
[2.153.0](supabase/auth@v2.152.0...v2.153.0)
(2024-06-04)


### Features

* add SAML specific external URL config
([supabase#1599](supabase#1599))
([b352719](supabase@b352719))
* add support for verifying argon2i and argon2id passwords
([supabase#1597](supabase#1597))
([55409f7](supabase@55409f7))
* make the email client explicity set the format to be HTML
([supabase#1149](supabase#1149))
([53e223a](supabase@53e223a))


### Bug Fixes

* call write header in write if not written
([supabase#1598](supabase#1598))
([0ef7eb3](supabase@0ef7eb3))
* deadlock issue with timeout middleware write
([supabase#1595](supabase#1595))
([6c9fbd4](supabase@6c9fbd4))
* improve token OIDC logging
([supabase#1606](supabase#1606))
([5262683](supabase@5262683))
* update contributing to use v1.22
([supabase#1609](supabase#1609))
([5894d9e](supabase@5894d9e))

---
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
@kangmingtay @LashaJini
fix: improve token OIDC logging (supabase#1606)
dbf20d9 
## What kind of change does this PR introduce?
* Currently, when the "Unacceptable audience in id_token" error is
returned, it doesn't log the audience claim from the id token, which
makes it hard to debug. The audience claim from the id token is now
logged as well when this error is returned.
* Adds a basic test for the generic id token oidc `getProvider()`
method, since we currently have 0 coverage for this file
* The test also uncovered a possible nil pointer panic in the case of
the generic OIDC provider being returned since in the generic case, the
`oauthConfig` will be nil. Rather than returning the `oauthConfig`, we
only need to return the `skipNonceCheck` property since we only check
for that.
LashaJini pushed a commit to LashaJini/auth that referenced this pull request Nov 15, 2024
@github-actions @LashaJini
chore(master): release 2.153.0 (supabase#1596)
299c61b 
🤖 I have created a release *beep* *boop*
---


##
[2.153.0](supabase/auth@v2.152.0...v2.153.0)
(2024-06-04)


### Features

* add SAML specific external URL config
([supabase#1599](supabase#1599))
([b352719](supabase@b352719))
* add support for verifying argon2i and argon2id passwords
([supabase#1597](supabase#1597))
([55409f7](supabase@55409f7))
* make the email client explicity set the format to be HTML
([supabase#1149](supabase#1149))
([53e223a](supabase@53e223a))


### Bug Fixes

* call write header in write if not written
([supabase#1598](supabase#1598))
([0ef7eb3](supabase@0ef7eb3))
* deadlock issue with timeout middleware write
([supabase#1595](supabase#1595))
([6c9fbd4](supabase@6c9fbd4))
* improve token OIDC logging
([supabase#1606](supabase#1606))
([5262683](supabase@5262683))
* update contributing to use v1.22
([supabase#1609](supabase#1609))
([5894d9e](supabase@5894d9e))

---
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

@J0 J0 J0 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
@kangmingtay @coveralls @J0