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

PAYARA-3825 Validity check and auto-refresh for OpenID connect tokens. #3922

Merged
merged 7 commits into from Jul 23, 2019

Conversation

@parysto
Copy link
Contributor

commented Apr 30, 2019

Added (optional) validity-check for Access Tokens and automated refresh of Access and Refresh tokens to OpenID connect authentication mechanism.

If auto-refresh is enabled but the Refresh Token is invalid (expired or user has logged out from OpenID connect provider), (s)he is logged out from the application (app-server) as well and redirected to the OpenID Connect provider. This allows the user to login again.

Validity check and auto-refresh for OpenID connect tokens.
Added (optional) validity-check for Access Tokens and automated refresh of Access and Refresh tokens to OpenID connect authentication mechanism.

@MarkWareham MarkWareham requested a review from jGauravGupta May 1, 2019

@Pandrex247 Pandrex247 changed the title Validity check and auto-refresh for OpenID connect tokens. PAYARA-3825 Validity check and auto-refresh for OpenID connect tokens. May 20, 2019

@Pandrex247 Pandrex247 modified the milestone: 5.193 May 20, 2019

@parysto

This comment has been minimized.

Copy link
Contributor Author

commented May 27, 2019

@jGauravGupta: Please review the changes I have committed to fix your findings.

@jGauravGupta

This comment has been minimized.

Copy link
Contributor

commented May 28, 2019

Hi @parysto,

I have successfully tested the PR with Google & Azure OIDC provider with few changes and I will create the PR for Google & Azure OIDC provider auto-refresh support.

Also, I think tokenAutoRefresh default value should be set to false.

@parysto

This comment has been minimized.

Copy link
Contributor Author

commented May 28, 2019

Hi @jGauravGupta,

yes, I do think it makes sense to disable tokenAutoRefresh by default, so that applications using OpenID connect do not change their behaviour after updating Payara. I have updated the annotation.

@jGauravGupta jGauravGupta requested a review from arjantijms May 29, 2019

@jGauravGupta

This comment has been minimized.

Copy link
Contributor

commented May 29, 2019

@arjantijms your review needed as @javax.security.enterprise.authentication.mechanism.http.AutoApplySession annotation removed in PR.

@jGauravGupta

This comment has been minimized.

Copy link
Contributor

commented Jun 5, 2019

Hi @parysto,

Thanks for the awesome work. Signed CLA is needed before any pull requests can be merged. Can you take a look at https://github.com/payara/Payara/blob/master/Contributing.md and get the CLA over to info@payara.fish.

@parysto

This comment has been minimized.

Copy link
Contributor Author

commented Jun 5, 2019

Signed CLA is needed before any pull requests can be merged.

Done :-)

@jGauravGupta jGauravGupta requested a review from MarkWareham Jun 5, 2019

@smillidge smillidge added CLA and removed Awaiting CLA labels Jun 7, 2019

@jGauravGupta

This comment has been minimized.

Copy link
Contributor

commented Jun 26, 2019

jenkins test please

@jGauravGupta jGauravGupta requested a review from Pandrex247 Jul 10, 2019

@Pandrex247
Copy link
Member

left a comment

I don't have a true test case but looks OK and has survived me building and poking it

@jGauravGupta jGauravGupta added this to the 5.193 milestone Jul 16, 2019

@arjantijms

This comment has been minimized.

Copy link
Contributor

commented Jul 18, 2019

@javax.security.enterprise.authentication.mechanism.http.AutoApplySession annotation removed in PR

Mechanisms can indeed do what AutoApplySession does themselves in a customised way. Would maybe be a good idea to study this case to see if the Jakarta Security API can accommodate such cases beter.

@arjantijms

This comment has been minimized.

Copy link
Contributor

commented Jul 18, 2019

Jenkins test please

@arjantijms arjantijms merged commit 13bd97b into payara:master Jul 23, 2019

2 of 6 checks passed

security/snyk - appserver/admingui/pom.xml (payara-ci) Test in progress
security/snyk - appserver/deployment/pom.xml (payara-ci) Test in progress
security/snyk - appserver/orb/pom.xml (payara-ci) Test in progress
security/snyk - appserver/registration/pom.xml (payara-ci) Test in progress
Payara Quick Build and Test Quick build and test passed!
Details
security/snyk - nucleus/test-utils/pom.xml (payara-ci) No new issues
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.