validate access token by ZhuXiaoBing-cn · Pull Request #17566 · Azure/azure-sdk-for-java

ZhuXiaoBing-cn · 2020-11-13T08:42:48Z

add resource server autoconfiguration
add audience validator
add tenant validator
add issuer validator

* add resource server autoconfiguration * add audience validator * add tenant validator * add issuer validator

…re#17530) * Upgrade msal.js to a higher version to use PKCE.

saragluna · 2020-11-13T08:45:29Z

...g-boot/src/main/java/com/azure/spring/aad/resource/AzureResourceServerAutoConfiguration.java

+@EnableConfigurationProperties(AADAuthenticationProperties.class)
+@ConditionalOnClass(name = {"org.springframework.security.oauth2.server.resource.BearerTokenAuthenticationToken"})
+@ConditionalOnWebApplication(type = ConditionalOnWebApplication.Type.SERVLET)
+@ConditionalOnProperty(prefix = "azure.activedirectory", value = {"client-id", "client-secret", "tenant-id"})


Why do we need these properties?

saragluna · 2020-11-13T08:47:25Z

...g-boot/src/main/java/com/azure/spring/aad/resource/AzureResourceServerAutoConfiguration.java

+    @ConditionalOnMissingBean(JwtDecoder.class)
+    JwtDecoder jwtDecoderByJwkKeySetUri() {
+        IdentityEndpoints endpoints = new IdentityEndpoints(aadAuthenticationProperties.getUri());
+        NimbusJwtDecoder nimbusJwtDecoder = NimbusJwtDecoder


I see why the tenant id is required here, but why this prefix azure.activedirectory.resource is used here?

So for the case of single-tenant app, the tenant id will be its tenant id. For multi-tenant app, it will be common?

about azure.activedirectory.resource ,a hand by mistake,it will be modified.
So for the case of single-tenant app, the tenant id will be its tenant id. For multi-tenant app, it will be common?
yes

saragluna · 2020-11-13T08:51:29Z

...g-boot/src/main/java/com/azure/spring/aad/resource/AzureResourceServerAutoConfiguration.java

+    private AADAuthenticationProperties aadAuthenticationProperties;
+
+    @Bean
+    @ConditionalOnProperty(prefix = "azure.activedirectory.resource", value = {"client-id", "tenant-id",


All these properties are not required right?

If tenant id is provided, we will construct the jwkset uri from the tenant id, otherwise we could use common?
If no client-id or app-id-uri is provided, we could skip the audience check?

All these properties are not required right?
yes.

If tenant id is provided, we will construct the jwkset uri from the tenant id, otherwise we could use common? If no client-id or app-id-uri is provided, we could skip the audience check?

Yes, you are right.

saragluna · 2020-11-13T08:54:11Z

...g-boot/src/main/java/com/azure/spring/aad/resource/AzureResourceServerAutoConfiguration.java

+        List<String> validAudiences = new ArrayList<>();
+        validAudiences.add(aadAuthenticationProperties.getClientId());
+        validAudiences.add(aadAuthenticationProperties.getAppIdUri());
+        validators.add(new AzureJwtIssuerValidator(aadAuthenticationProperties.getTenantId(),


Why are we using tenant ids to construct the issuer validator?

Because the issuer in the form of "https://sts.windows.net/4acebda4-c077-434a-8675-c4b5afebc8da/", so I hope that by passing tenantId to construct corresponding issuer, then the access token of the 'iss' of claims to match.

saragluna · 2020-11-13T08:57:01Z

...ring-boot/src/main/java/com/azure/spring/aad/resource/validator/AzureJwtIssuerValidator.java

+
+    private List<String> assembleIssuer(String tenantId) {
+        List<String> issuers = new ArrayList<>();
+        issuers.add(LOGIN_MICROSOFT_ONLINE_ISSUER + tenantId + "/");


Do we need to check the tenant in issuer claim in this validator? Which doc are we following here?

saragluna · 2020-11-13T09:17:33Z

...ring-boot/src/main/java/com/azure/spring/aad/resource/validator/AzureJwtTenantValidator.java

+    private final JwtClaimValidator<String> validator;
+
+    public AzureJwtTenantValidator(String tenantId, Set<String> allowedTenantids) {
+        Assert.notNull(tenantId, "tenantId cannot be null");


We need both to be non-null here?

Yes,It will be modified.

…earch to PhoneNumberReservation (Azure#17253) * Renaming from PhoneNumberSearch to PhoneNumberReservation * Renaming from PhoneNumberSearch to PhoneNumberReservation * Renaming from PhoneNumberSearch to PhoneNumberReservation * upadating readme samples * Renaming the model CreateSearchReponse and CreateSearchOptions * Fixing tests * Fixing tests * fixing typo un reservations

…e#17565) * Add AppConfig and Event Hubs samples for using exporters * Fix compiler warnings * Update sdk/monitor/microsoft-opentelemetry-exporter-azuremonitor/pom.xml * Update method names

* Improve Update-ChangeLog Logic * Updates to ChangeLog-Operations.ps1, copy-docs-to-blobstorage.ps1, Invoke-GitHubAPI.ps1 and Package-Properties.ps1 * More changeLog Logic Improvements * Update date parsing Co-authored-by: Chidozie Ononiwu <chononiw@microsoft.com>

… public to private (Azure#17576) * changing some public methods * updating Reade file * Fixing Readme

* [Service Bus] Remove viaPartitionKey * Remove unused imports

…be performed * tenantId is null, the default assignment is 'common' * The properties of AzureResourceServerAutoConfiguration made changes

…g-cn/azure-sdk-for-java into validate_access_token

jialindai · 2020-11-16T10:05:36Z

What will be the configuration file looks like?

jialindai · 2020-11-16T10:10:00Z

We also need unit tests.

* add unit tests * pom file add resource-server dependencies * spring.factories file add autoconfiguration

ZhuXiaoBing-cn · 2020-11-17T05:20:04Z

@jialindai

What will be the configuration file looks like?

configuration file looks like：
In the case of a single tenant, our application.properties:

azure.activedirectory.tenant-id=4acebda4-xxxxxx-c4b5afebc
azure.activedirectory.client-id=f6fae4ff-xxxxxx-ccb0a2ee2
azure.activedirectory.app-id-uri=api://f6fae4ff-xxxxxx-ccb0a2ee2

Configuration above, we will verify aud,iss,tid,nbf,exp.
When our azure.activedirectory.client-id and azure.activedirectory.app-id-uri are not configured, we will not validate the aud.when our azure.activedirectory.tenant-id are not configured,it will be given a default common,and we will skip tid and iss.For nbf,exp,it is validated by default.

In the case of a multi-tenant, our application.properties:

azure.activedirectory.tenant-id=common
azure.activedirectory.client-id=f6fae4ff-xxxxxx-ccb0a2ee2e2c
azure.activedirectory.app-id-uri=api://f6fae4ff-xxxxxx-ccb0a2ee2e2c
azure.activedirectory.allowed-tenant-ids=4acebda4-xxxxxx-c4b58da,3aef70d5-xxxxxx-22e8a91,308e3453-xxxxxx-28a8fb5e

Under multi-tenant the azure.activedirectory.tenant-id will be configured as common and if it is not configured he will use common by default.For azure.activedirectory.allowed-tenant-ids, the configuration will be available to those tenants.When it is configured, we will validate tid, iss, otherwise we will not validate.

We also need unit tests.
This PR, I moved validate access token #17613 and I've added unit test.

validate access token

f8f2940

* add resource server autoconfiguration * add audience validator * add tenant validator * add issuer validator

ZhuXiaoBing-cn requested review from backwind1233, chenrujun, jialindai, saragluna and yiliuTo as code owners November 13, 2020 08:42

ghost added the azure-spring All azure-spring related issues label Nov 13, 2020

saragluna added the azure-spring-aad Spring active directory related issues. label Nov 13, 2020

Change the method of obtaining tokens from implicit flow to pkce (Azu…

65b9871

…re#17530) * Upgrade msal.js to a higher version to use PKCE.

saragluna requested changes Nov 13, 2020

View reviewed changes

paolamvhz and others added 10 commits November 13, 2020 12:59

Add AppConfig and Event Hubs samples for using Monitor exporter (Azur…

88e5368

…e#17565) * Add AppConfig and Event Hubs samples for using exporters * Fix compiler warnings * Update sdk/monitor/microsoft-opentelemetry-exporter-azuremonitor/pom.xml * Update method names

Update amqp version post-1.7.0-beta.2 release (Azure#17577)

559a8c1

[Communication] -Administration- changing some createReservation from…

1eaa75d

… public to private (Azure#17576) * changing some public methods * updating Reade file * Fixing Readme

November release changelog (Azure#17571)

7942159

[Service Bus] Remove viaPartitionKey (Azure#17501)

70d49f4

* [Service Bus] Remove viaPartitionKey * Remove unused imports

Enable the del/rename files link check. (Azure#17574)

7f0b710

* clientId and AppIdUri are both null, audience validation will not …

fca48d1

…be performed * tenantId is null, the default assignment is 'common' * The properties of AzureResourceServerAutoConfiguration made changes

Merge branch 'validate_access_token' of https://github.com/ZhuXiaoBin…

af9071d

…g-cn/azure-sdk-for-java into validate_access_token

* Make changes according to checkstyle

ca6ea29

* add unit tests * pom file add resource-server dependencies * spring.factories file add autoconfiguration

ZhuXiaoBing-cn requested review from alzimmermsft, ankitarorabit, anuchandy, bsiegel, conniey, danieljurek and hemanttanwar as code owners November 17, 2020 03:30

ZhuXiaoBing-cn requested review from a team, JianpingChen, JimSuplizio, JonathanGiles, RezaJooyandeh, YijunXieMS, jianghaolu, joshfree, minnieliu, mitchdenny, samvaity, srnagar, weshaggard and yvgopal as code owners November 17, 2020 03:30

ZhuXiaoBing-cn closed this Nov 17, 2020

Conversation

ZhuXiaoBing-cn commented Nov 13, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jialindai commented Nov 16, 2020

Uh oh!

jialindai commented Nov 16, 2020

Uh oh!

ZhuXiaoBing-cn commented Nov 17, 2020 • edited by saragluna Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

ZhuXiaoBing-cn commented Nov 17, 2020 •

edited by saragluna

Loading