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: JWT token will always be used for user info, expiry and verification #243

Merged
merged 2 commits into from
Aug 4, 2022
Merged

feat: JWT token will always be used for user info, expiry and verification #243

merged 2 commits into from
Aug 4, 2022

Conversation

DeepDiver1975
Copy link
Member

Description

Whenever the given token is a JWT we try to use it for:

  • user information
  • expiry
  • verification

Motivation and Context

In some scenarios non JWT and JWT tokens can be used in parallel. We support this now.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised:

Open tasks:

  • ...

@DeepDiver1975
feat: if token is no JWT fallback to user info endpoint
7e2e586 
feat: if the token is a JWT it will always be used to verify and get the expiry - even if token introspection is enabled
@DeepDiver1975 DeepDiver1975 force-pushed the feat/token-exchange-and-self-signed-jwt-support-2 branch 2 times, most recently from 11fd52a to a506569 Compare July 18, 2022 13:30
@DeepDiver1975
Copy link
Member Author

@mmattel once this is merged the config setting 'use-token-introspection-endpoint' is to be removed from the docs

@mmattel
Copy link
Contributor

mmattel commented Jul 18, 2022

Will that PR finish "soon"?
because we are doing QA on many new app releases including openidconect 😄

@DeepDiver1975
Copy link
Member Author

Will that PR finish "soon"?
because we are doing QA on many new app releases including openidconect smile

This is not to be added to the next release.

@DeepDiver1975
Copy link
Member Author

refs #241 (comment)

@mmattel
Copy link
Contributor

mmattel commented Jul 18, 2022

Thanks for clarification 👍

@DeepDiver1975 DeepDiver1975 force-pushed the feat/token-exchange-and-self-signed-jwt-support-2 branch from a506569 to af61681 Compare August 2, 2022 08:07
@DeepDiver1975
Copy link
Member Author

@mmattel once this is merged the config setting 'use-token-introspection-endpoint' is to be removed from the docs

also use-access-token-payload-for-user-info will go @mmattel

@sonarqubecloud
Copy link

sonarqubecloud bot commented Aug 2, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 3 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@mmattel mmattel requested review from butonic, IljaN and phil-davis August 2, 2022 15:30
@jnweiger
Copy link
Contributor

jnweiger commented Aug 2, 2022 •
edited
Loading

This is not to be added to the next release

Why not?
openidconnect-2.2.0 is ready for QA. But you can still add this PR.

@mmattel
Copy link
Contributor

mmattel commented Aug 2, 2022

Regarding the config.sample changes, these need to go into core and then transported to docs. I will file an issue in docs and a core pr.

@mmattel mmattel mentioned this pull request Aug 2, 2022
Closed
12 tasks
@jnweiger jnweiger mentioned this pull request Aug 2, 2022
Closed
42 tasks
@mmattel mmattel mentioned this pull request Aug 2, 2022
Merged
11 tasks
@DeepDiver1975
Copy link
Member Author

Why not?
openidconnect-2.2.0 is ready for QA. But you can still add this PR.

Well - 15 days ago the status of this all was different and I wanted to make sure that nothing breaks ....

IljaN
IljaN approved these changes Aug 4, 2022
View reviewed changes
Copy link
Member

@IljaN IljaN left a comment

Choose a reason for hiding this comment

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

LGTM 👯

@jnweiger jnweiger merged commit af0ff59 into master Aug 4, 2022
@delete-merged-branch delete-merged-branch bot deleted the feat/token-exchange-and-self-signed-jwt-support-2 branch August 4, 2022 10:04
mirekys referenced this pull request in CESNET/cesnet-openidconnect Sep 5, 2022
@mirekys
Revert "Merge pull request #243 from owncloud/feat/token-exchange-and…
663660d 
…-self-signed-jwt-support-2"

This reverts commit af0ff59, reversing
changes made to 1994c40.
mirekys referenced this pull request in CESNET/cesnet-openidconnect Sep 6, 2022
@mirekys
Merge pull request #18 from CESNET/rollback-token-code
132e737 
Revert "Merge pull request #243 from owncloud/feat/token-exchange-and…
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@IljaN IljaN IljaN approved these changes

@butonic butonic Awaiting requested review from butonic

@phil-davis phil-davis Awaiting requested review from phil-davis

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

Successfully merging this pull request may close these issues.
4 participants
@DeepDiver1975 @mmattel @jnweiger @IljaN