Add OIDC issuer validation and new testing style#970
Conversation
msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AbstractClientApplicationBase.java
Outdated
Show resolved
Hide resolved
msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/OidcAuthority.java
Outdated
Show resolved
Hide resolved
| OidcDiscoveryProvider.performOidcDiscovery( | ||
| (OidcAuthority) authenticationAuthority, this)); | ||
|
|
||
| if (!((OidcAuthority) authenticationAuthority).isIssuerValid()) { |
There was a problem hiding this comment.
This upcasting is a code smell. If you use inheritance, have you considered adding isIssuerValid to the base class Authority, where it simply returns true and have custom logic in OidcAuthority ?
There was a problem hiding this comment.
In the latest commit I rearranged the methods: the call to the validation method now happens in the setAuthorityProperties() method, so there are now no changes to this class and all the logic is contained within OidcAuthority
msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AbstractClientApplicationBase.java
Outdated
Show resolved
Hide resolved
msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/testcase/Act.java
Outdated
Show resolved
Hide resolved
msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/RunnerTest.java
Outdated
Show resolved
Hide resolved
bgavrilMS
left a comment
There was a problem hiding this comment.
The test infra is not ready. Maybe have a regular test instead?
In the latest commit I removed the POC test infrastructure and added a new OidcAuthorityTest class to handle the tests in a more traditional way. I had mainly added it to this PR to more easily get reviews/feedback on the new style, and I'll make a follow-up PR for those changes. |
This PR adds issuer validation for OIDC scenarios: 3268767
oidcAuthority()API, a call is made to the OIDC discovery endpoint based on the authority set for the applicationIn addition, to test the issuer validation this PR builds on the work done in #968 to add a new testing style which was first proposed in MSAL Python: AzureAD/microsoft-authentication-library-for-python#825
RunnerTestclass containing barebones test outlines for a few scenarios, including issuer validationRunnerHelperclass containing helper methods forRunnerTest: retrieve test cases from a central server, instantiate objects/execute methods/validate results all based on the test configtestcasefolder which represent and parses the content of test cases from the central server