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

TEST-198 Refactor JsonWebToken API in MP TCK Runner JWT Auth #4054

Merged
merged 10 commits into from Jul 4, 2019

Conversation

@AlanRoth
Copy link
Contributor

commented Jun 17, 2019

Moved TokenParser class to Payara Server from MP TCK, plus changes to make it compatible with #3799 which is mostly the same changes #3799 does to the Parse method.

Relies on #44 in MicroProfile-TCK-Runners
payara/MicroProfile-TCK-Runners#44

AlanRoth added 3 commits Jun 7, 2019
sync
@AlanRoth

This comment has been minimized.

Copy link
Contributor Author

commented Jun 17, 2019

jenkins test please

}

public JsonWebTokenImpl verify(String issuer, PublicKey publicKey) throws Exception {
SignedJWT signedJWT = SignedJWT.parse(rawToken);

This comment has been minimized.

Copy link
@cubastanley

cubastanley Jun 18, 2019

Contributor

Don't feel great about the ambiguity of this local variable having the same name as the signedJWT variable at line 76 - could it have a slightly different name?

This comment has been minimized.

Copy link
@AlanRoth

AlanRoth Jun 18, 2019

Author Contributor

signedJWT2? I don't think its worth the change as #3799 is refactoring this class, I just did the minimum to make it function for now with the least amount of changes; #3799 is due for a merge shortly after this.

This comment has been minimized.

Copy link
@AlanRoth

AlanRoth Jun 18, 2019

Author Contributor

That line can be removed and a null check added - as you should call parse before calling verify

@AlanRoth AlanRoth requested a review from cubastanley Jun 19, 2019
@AlanRoth

This comment has been minimized.

Copy link
Contributor Author

commented Jun 19, 2019

jenkins test please

AlanRoth added 2 commits Jul 3, 2019
@AlanRoth AlanRoth requested review from jGauravGupta, MattGill98, Cousjava and cubastanley and removed request for Pandrex247 and MarkWareham Jul 3, 2019
@AlanRoth AlanRoth removed the DO NOT MERGE label Jul 3, 2019
@AlanRoth

This comment has been minimized.

Copy link
Contributor Author

commented Jul 3, 2019

jenkins test please

Copy link
Contributor

left a comment

Agreed with Matt's requests, otherwise I'm happy

Co-Authored-By: Matt Gill <MattGill98@users.noreply.github.com>
@AlanRoth AlanRoth requested a review from MattGill98 Jul 4, 2019
@AlanRoth

This comment has been minimized.

Copy link
Contributor Author

commented Jul 4, 2019

Agreed with Matt's requests, otherwise I'm happy

@cubastanley I agree too, and I had a chat with Matt and #3799 will revert any changes back to using those two variables, which is a bit of a conundrum, it wouldn't be too bad of a shout on pointing it out on #3799

@AlanRoth

This comment has been minimized.

Copy link
Contributor Author

commented Jul 4, 2019

jenkins test please

@AlanRoth AlanRoth merged commit cd17fdf into payara:master Jul 4, 2019
59 checks passed
59 checks passed
Payara Quick Build and Test Quick build and test passed!
Details
security/snyk - api/payara-api/pom.xml (payara-ci) No new issues
Details
security/snyk - api/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/admin/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/admingui/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/ant-tasks/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/appclient/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/batch/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/common/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/concurrent/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/connectors/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/core/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/deployment/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/distributions/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/ejb/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/extras/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/featuresets/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/flashlight/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/grizzly/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/ha/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/installer/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/javaee-api/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/jdbc/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/jms/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/load-balancer/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/orb/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/osgi-platforms/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/packager/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/payara-appserver-modules/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/persistence/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/registration/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/resources/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/security/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/tests/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/transaction/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/web/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/webservices/pom.xml (payara-ci) No new issues
Details
security/snyk - copyright/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/admin/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/cluster/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/common/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/core/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/deployment/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/diagnostics/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/distributions/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/flashlight/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/grizzly/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/hk2/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/osgi-platforms/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/packager/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/payara-modules/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/resources-l10n/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/resources/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/security/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/test-utils/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/tests/pom.xml (payara-ci) No new issues
Details
security/snyk - pom.xml (payara-ci) No new issues
Details
@arjantijms arjantijms added this to the 5.193 milestone Jul 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.