-
Notifications
You must be signed in to change notification settings - Fork 135
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
Update JwtBearerTokenFlow.java #1346
Conversation
Added additional setter for an opaque token responses. Signed-off-by: Kalin Borisov <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi Kalin, thanks for your contribution.
Can you please make the following changes?
- change method to "setOpaqueTokenFormat(boolean opaque)" that allows both enabling or disabling the opaque format similiar to disableCache(boolean) that is already there.
- add a unit test here for the new method that verifies that the optionalParameters contains the correct setting after using the method
- add a constant for "token_format" similar to "token_type" here: https://github.com/SAP/cloud-security-xsuaa-integration/blob/e5f4196a9f88d207b43664e421f88bbf56969d06/token-client/src/main/java/com/sap/cloud/security/xsuaa/client/OAuth2TokenServiceConstants.java#L17
- use the constant in your code and in addition in the two places where "token_format" has been hard-coded before: https://github.com/SAP/cloud-security-xsuaa-integration/blob/e5f4196a9f88d207b43664e421f88bbf56969d06/token-client/src/test/java/com/sap/cloud/security/xsuaa/client/XsuaaOAuth2TokenServiceJwtBearerTokenTest.java#L131
https://github.com/SAP/cloud-security-xsuaa-integration/blob/e5f4196a9f88d207b43664e421f88bbf56969d06/token-client/src/test/java/com/sap/cloud/security/xsuaa/client/XsuaaOAuth2TokenServicePasswordTest.java#L146
Thanks!
Manuel
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your changes. Looks very good. Just one small thing. Then I would like to merge it:
Can you please add one more assertion to the unit test that by default (if setOpaqueTokenFormat was not called yet), it does not contain the token_format key = opaque entry?
And likewise, at the end of the method, add the same assertion after calling setOpaqueTokenFormat(false) after having called setOpaqueTokenFormat(true) before?
09e9806
to
3af4fc1
Compare
3af4fc1
to
ef0fb56
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perfect, thank you for the adjustments!
JwtBearerTokenFlow: add additional setter to request an opaque token response Co-authored-by: Kalin Borisov <[email protected]>
Added additional setter for an opaque token responses.