-
Notifications
You must be signed in to change notification settings - Fork 177
Conversation
}); | ||
getProfile() { | ||
|
||
this.http.get(protectedResources.graphMe.endpoint).subscribe( |
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.
let's move the http client into the service file (graph.service). the component shouldn't do http calls by itself
|
||
addClaimsToStorage( | ||
claimsChallenge, | ||
`cc.${msalConfig.auth.clientId}.${account?.idTokenClaims?.oid}` |
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.
should the key schema also contain something to identify which resource this claims challenge is for?
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.
Looks good to me -couple of comments to address, but it's working fine. thanks @salman90
PS prints the following, please remove
|
@derisen , please add a section about cliant capabilities and code handling CAE (as explained in https://github.com/Azure-Samples/active-directory-aspnetcore-webapp-openidconnect-v2/blob/master/2-WebApp-graph-user/2-1-Call-MSGraph/README.md#optional---handle-continuous-access-evaluation-cae-challenge-from-microsoft-graph) in the following tow readme and the code generator
In the codegen, please append a task to generate a stub about CAE whenever the "service" in sample.json is MS Graph. Thanks |
pressing the login button throws the following error getPrototypeOf.js:5980 ERROR ServerError: invalid_request: 9002326 - [2022-08-17 11:05:16Z]: AADSTS9002326: Cross-origin token redemption is permitted only for the 'Single-Page Application' client-type. Request origin: 'http://localhost:4200'. |
This is because the redirect Uri is not of type SPA. The scripts are using Azure AD module, which can't register a SPA, so it needs to be manually changed for now. |
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.
Purpose
Does this introduce a breaking change?
Pull Request Type
What kind of change does this Pull Request introduce?
How to Test
What to Check
Verify that the following are valid
Other Information