Add azp to JWKS endpoint headers #1285

finkmanAtSap · 2023-09-11T14:41:32Z

No description provided.

.../src/main/java/com/sap/cloud/security/token/validation/validators/JwtSignatureValidator.java

.../java/com/sap/cloud/security/token/validation/validators/OAuth2TokenKeyServiceWithCache.java

…ew method a default method

…ecated

.../java/com/sap/cloud/security/token/validation/validators/OAuth2TokenKeyServiceWithCache.java

liga-oz

Some things to consider, see comments.

.../src/main/java/com/sap/cloud/security/token/validation/validators/JwtSignatureValidator.java

...security/src/main/java/com/sap/cloud/security/token/validation/validators/JsonWebKeySet.java

.../java/com/sap/cloud/security/token/validation/validators/OAuth2TokenKeyServiceWithCache.java

…or and SapIdJwtSignatureValidator

… hotfix/jwksEndpointHeaders

finkmanAtSap · 2023-10-02T14:45:04Z

Split JwtSignatureValidator into two service-specific implementations to clean up its code and prepare for service-specific retrieval of JWKS in future refactorings.

liga-oz

All in all really nice job 💯
I'm just troubled by the number of arguments that TokenKeyService method retrieveTokenKeys and OAuth2TokenKeyServiceWithCache.getPublicKey(JwtSignatureAlgorithm keyAlgorithm, String keyId, URI keyUri, String appTid, String clientId, String azp) introduces, it should be refactored and a new class should be introduced that encapsulates all the required header parameters.

.../java/com/sap/cloud/security/token/validation/validators/OAuth2TokenKeyServiceWithCache.java

...main/java/com/sap/cloud/security/token/validation/validators/XsuaaJwtSignatureValidator.java

token-client/src/main/java/com/sap/cloud/security/xsuaa/client/SpringOAuth2TokenKeyService.java

token-client/src/main/java/com/sap/cloud/security/xsuaa/client/OAuth2TokenKeyService.java

...n-client/src/main/java/com/sap/cloud/security/xsuaa/client/DefaultOAuth2TokenKeyService.java

…icit IAS-specific parameters

finkmanAtSap · 2023-10-12T08:58:43Z

All in all really nice job 💯 I'm just troubled by the number of arguments that TokenKeyService method retrieveTokenKeys and OAuth2TokenKeyServiceWithCache.getPublicKey(JwtSignatureAlgorithm keyAlgorithm, String keyId, URI keyUri, String appTid, String clientId, String azp) introduces, it should be refactored and a new class should be introduced that encapsulates all the required header parameters.

I used a Map<String, String> now to pass the parameters since the required parameters are different for XSUAA and IAS and I do not think a dedicated class can be defined with the required parameters that works for both services.

...n-client/src/main/java/com/sap/cloud/security/xsuaa/client/DefaultOAuth2TokenKeyService.java

@@ -77,7 +67,7 @@
 							.build();
 				}

-				LOGGER.debug("Successfully retrieved token keys from {} for tenant '{}'", tokenKeysEndpointUri, tenantId);
+				LOGGER.debug("Successfully retrieved token keys from {} with params {}.", tokenKeysEndpointUri, params);


token-client/src/main/java/com/sap/cloud/security/xsuaa/client/SpringOAuth2TokenKeyService.java

 		try {
 			ResponseEntity<String> response = restOperations.exchange(
 					tokenKeysEndpointUri, GET, new HttpEntity<>(headers), String.class);
 			if (HttpStatus.OK.value() == response.getStatusCode().value()) {
-				LOGGER.debug("Successfully retrieved token keys from {} for tenant '{}'", tokenKeysEndpointUri, tenantId);
+				LOGGER.debug("Successfully retrieved token keys from {} for params '{}'", tokenKeysEndpointUri, params);


liga-oz

lgtm, so much cleaner with the map as param 👍🏻 just check the 2 comments before merging.

...main/java/com/sap/cloud/security/token/validation/validators/XsuaaJwtSignatureValidator.java

token-client/src/main/java/com/sap/cloud/security/xsuaa/client/testa.java

* add x-azp header to JWKS fetching and adjust JWKS cache key * refactor JwtSignatureValidator -> Split into XsuaaJwtSignatureValidator and SapIdJwtSignatureValidator * refactor OAuth2TokenKeyService and OAuth2TokenKeyServiceWithCache APIs to use generic Map instead of explicit IAS-specific parameters --------- Co-authored-by: liga-oz <liga.ozolina@sap.com>

finkmanAtSap added 2 commits September 11, 2023 16:08

add x-azp header constant

18a8721

add x-azp header to JWKS fetching and adjust JWKS caching

ac41268

finkmanAtSap requested a review from liga-oz September 11, 2023 14:41

finkmanAtSap commented Sep 11, 2023

View reviewed changes

.../src/main/java/com/sap/cloud/security/token/validation/validators/JwtSignatureValidator.java Outdated Show resolved Hide resolved

github-advanced-security bot found potential problems Sep 11, 2023

View reviewed changes

finkmanAtSap and others added 5 commits September 12, 2023 09:55

Merge branch 'main' into hotfix/jwksEndpointHeaders

0b9dcc7

make OAuth2TokenKeyService interface functional again by making its n…

7d4f83a

…ew method a default method

add missing @OverRide annotation

d4121e8

Bugfix for JWKS CacheKey toString

ed7c8b4

Continue setting JsonWebKeySet appTid collection but mark API as depr…

896d550

…ecated

github-advanced-security bot found potential problems Sep 12, 2023

View reviewed changes

.../java/com/sap/cloud/security/token/validation/validators/OAuth2TokenKeyServiceWithCache.java Fixed Show fixed Hide fixed

liga-oz requested changes Sep 14, 2023

View reviewed changes

.../src/main/java/com/sap/cloud/security/token/validation/validators/JwtSignatureValidator.java Outdated Show resolved Hide resolved

.../src/main/java/com/sap/cloud/security/token/validation/validators/JwtSignatureValidator.java Outdated Show resolved Hide resolved

finkmanAtSap requested a review from mwdb September 25, 2023 12:45

mwdb reviewed Sep 25, 2023

View reviewed changes

...security/src/main/java/com/sap/cloud/security/token/validation/validators/JsonWebKeySet.java Outdated Show resolved Hide resolved

mwdb reviewed Sep 25, 2023

View reviewed changes

.../java/com/sap/cloud/security/token/validation/validators/OAuth2TokenKeyServiceWithCache.java Outdated Show resolved Hide resolved

liga-oz mentioned this pull request Sep 26, 2023

Support app_tid SAP/cloud-security-client-go#94

Merged

finkmanAtSap and others added 9 commits September 27, 2023 11:23

Merge branch 'main' into hotfix/jwksEndpointHeaders

ec8752a

remove app_tid from JsonWebKeySet

f0c795e

Refactor JwtSignatureValidator -> Split into XsuaaJwtSignatureValidat…

a3b0015

…or and SapIdJwtSignatureValidator

Merge branch 'main' into hotfix/jwksEndpointHeaders

cc5094d

Harmonize JsonWebKeyConstants names

438e708

Merge remote-tracking branch 'origin/hotfix/jwksEndpointHeaders' into…

69b5979

… hotfix/jwksEndpointHeaders

Update Javadoc

296c445

Remove duplicate error message

12acd0d

Fix expected error message in Unit test

2b5e1f9

finkmanAtSap requested review from mwdb and liga-oz October 2, 2023 14:57

Bugfixes

9df22e0

liga-oz requested changes Oct 5, 2023

View reviewed changes

liga-oz and others added 4 commits October 5, 2023 17:41

add javadoc

dea7cb8

Merge branch 'main' into hotfix/jwksEndpointHeaders

b832613

Use ServiceConstant.XSUAA.VERIFICATION_KEY in XsuaaJwtSignatureValidator

05d4515

Refactor OAuth2TokenKeyService API to use generic Map instead of expl…

d313fdf

…icit IAS-specific parameters

finkmanAtSap closed this Oct 12, 2023

finkmanAtSap reopened this Oct 12, 2023

github-advanced-security bot found potential problems Oct 12, 2023

View reviewed changes

finkmanAtSap requested a review from liga-oz October 12, 2023 09:47

liga-oz approved these changes Oct 13, 2023

View reviewed changes

...main/java/com/sap/cloud/security/token/validation/validators/XsuaaJwtSignatureValidator.java Outdated Show resolved Hide resolved

token-client/src/main/java/com/sap/cloud/security/xsuaa/client/testa.java Outdated Show resolved Hide resolved

finkmanAtSap and others added 4 commits October 13, 2023 13:49

Remove accidentally committed file testa.java

90332ed

Refactor XsuaaJwtSignatureValidator fallback key logic

4936343

Fix for Refactor XsuaaJwtSignatureValidator fallback key logic

42fe0f4

Merge branch 'main' into hotfix/jwksEndpointHeaders

523613e

finkmanAtSap merged commit 07895cf into main Oct 13, 2023

finkmanAtSap deleted the hotfix/jwksEndpointHeaders branch October 13, 2023 12:19

finkmanAtSap mentioned this pull request Oct 16, 2023

Add azp header for Identity JWKS Retrieval #1312

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add azp to JWKS endpoint headers #1285

Add azp to JWKS endpoint headers #1285

finkmanAtSap commented Sep 11, 2023

liga-oz left a comment

finkmanAtSap commented Oct 2, 2023

liga-oz left a comment

finkmanAtSap commented Oct 12, 2023

liga-oz left a comment

Add azp to JWKS endpoint headers #1285

Add azp to JWKS endpoint headers #1285

Conversation

finkmanAtSap commented Sep 11, 2023

liga-oz left a comment

Choose a reason for hiding this comment

finkmanAtSap commented Oct 2, 2023

liga-oz left a comment

Choose a reason for hiding this comment

finkmanAtSap commented Oct 12, 2023

liga-oz left a comment

Choose a reason for hiding this comment