Skip to content

Conversation

@abhidnya13
Copy link
Contributor

Resolves #160

// todo: check for other authority types...
authorityType := "MSSTS"

var authorityType string
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do this in resolvers.go here
We delay doing this here because we want to avoid making a network call unless absolutely needed. So this code wont run while initializing app object here.

err := c.Comm.JSONCall(
ctx,
endpoint,
// TODO(jdoak): not thrilled about this, because all calls should have this but
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we add Correlation Id to all the calls that are part of a user flow for telemetry. I see thats what we do in MSAL Python. It was added here : AzureAD/microsoft-authentication-library-for-python#103
Could not find a spec though. @henrik-me thoughts ?

@abhidnya13 abhidnya13 marked this pull request as ready for review February 12, 2021 00:16
}

return resp.TenantDiscoveryEndpoint, nil
} else if authorityInfo.Tenant == "adfs" {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you make this this first condition in the if block you can remove the != check that's currently on line 126.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Mar 2, 2021

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@abhidnya13 abhidnya13 requested a review from jhendrixMSFT March 2, 2021 02:05
@abhidnya13 abhidnya13 merged commit db583aa into dev Mar 4, 2021
@abhidnya13 abhidnya13 deleted the authority_validation_todo branch March 4, 2021 21:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[TODO]Clean-up authority validation

3 participants