Skip to content
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

[Feature Request] Add dSTS support #467

Open
bgavrilMS opened this issue Dec 7, 2023 · 4 comments · May be fixed by #482
Open

[Feature Request] Add dSTS support #467

bgavrilMS opened this issue Dec 7, 2023 · 4 comments · May be fixed by #482
Assignees
Labels
enhancement New feature or request

Comments

@bgavrilMS
Copy link
Member

bgavrilMS commented Dec 7, 2023

To support a new authority type, we need to evolve MSAL.GO to have a better abstraction for authorities. Today, MSAL.GO supports AAD and ADFS authorities.

How MSAL detects the authority type

MSAL parses the authority string to figure out the authority type. If the string ends in "adfs" it is an indication that it is ADFS authority.

For dSTS, there is also specific logic to figure this out - the first path segment is hardcoded to "dstsv2", e.g. "https://some.url.dsts.core.azure-test.net/dstsv2/tenantID"

Endpoint discovery

Unlike MSAL.NET, MSAL GO will actually figure out the token_endpoint and authorize_endpoint from the STS, via OIDC discovery (@chlowell to keep me honest). Some logic needs to be added for dSTS. Note that MSAL.NET hardcodes the endpoints.

https://github.com/AzureAD/microsoft-authentication-library-for-go//blob/cc89d3480721852bd00435ac0ce220d12c2cc8a3/apps/internal/oauth/resolvers.go#L122

Authority validation

Today, AAD authorities are validated through instance discovery. ADFS authorities are not validated. dSTS authorities are not validated (but we might define some rules later). Ideally we should have an abstraction for this.

WithTenant

MSAL Go allows app developers to update the authority via the WithTenant API. See https://github.com/AzureAD/microsoft-authentication-library-for-go//blob/cc89d3480721852bd00435ac0ce220d12c2cc8a3/apps/internal/oauth/ops/authority/authority.go#L238

The logic must be updated to account for dSTS.

Acceptance Tests

We do not have E2E testing env for dSTS, so we have to rely on manual testing and unit testing.

  1. Get a token for client_credentials flow from dSTS, using both "https://some.url.dsts.core.azure-test.net/dstsv2/tenantID" and "https://some.url.dsts.core.azure-test.net/dstsv2/common" authority. Rely on a mocked HTTP response. Please ping me for this response sample. Ensure instance discovery is not done.

  2. As above, but change the tenant via WithTenant(T2). Ensure communication is now made over T2 endpoint.

  3. As 1, but make another request - ensure token comes from cache. Assert tokens cache keys.

@bgavrilMS bgavrilMS added the enhancement New feature or request label Dec 7, 2023
@chlowell
Copy link
Collaborator

Unlike MSAL.NET, MSAL GO will actually figure out the token_endpoint and authorize_endpoint from the STS, via OIDC discovery (@chlowell to keep me honest)

Yes, that's the design:

// ResolveEndpoints gets the authorization and token endpoints and creates an AuthorityEndpoints instance
func (m *authorityEndpoint) ResolveEndpoints(ctx context.Context, authorityInfo authority.Info, userPrincipalName string) (authority.Endpoints, error) {

@handsomejack-42
Copy link
Collaborator

I volunteer as tribute

@handsomejack-42 handsomejack-42 self-assigned this Jan 8, 2024
@handsomejack-42
Copy link
Collaborator

Hey @bgavrilMS, I'm finally able to prioritize working on this issue (yay).

I have made some changes, now I'm looking at Acceptance Tests section. I wanted to ask - is there an existing test I should mirror? Could you provide the link? I have been searching for client_credentials (as per Get a token for client_credentials flow from dSTS), but tests referencing that do not seem to really cover what I am implementing.

In other words, the goal is to mock the response from dSTS endpoint using a mocked http server, then proceed with test cases mentioned in 1, 2, 3 re the Acceptance Tests section? in context of method calls, I'm going to do AcquireTokenByCredential, is that sufficient? Should I call even AcquireTokenSilent for example?

Thanks.

@bgavrilMS
Copy link
Member Author

bgavrilMS commented Apr 10, 2024

For test 1, have a look at https://github.com/AzureAD/microsoft-authentication-library-for-go/blob/main/apps/confidential/confidential_test.go#L114

You should assert that:

  • you get the token
  • token caching works (i.e. via AcquireTokenSilent)
  • serialize the token cache (e.g. to JSON) and assert on that.

This is for an SPN token. I am not sure you want to enable user auth.

@handsomejack-42 handsomejack-42 linked a pull request Apr 15, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants