Skip to content
This repository was archived by the owner on Nov 21, 2024. It is now read-only.

Conversation

@glennamanns
Copy link
Contributor

Token integration tests rely on new appsettings.json file. The valid token path requires a proper client ID, client secret, issuer, and hostname. These tests focus on the token from client credentials path currently. We can add more once we decide on which token retrieval methods we want to expose for the GA token binding.

This PR also allows for the EasyAuthTokenClient to use HTTP to call /.auth endpoints. This is only useful/allowed during local testing.

@glennamanns
Copy link
Contributor Author

I can't add reviewers for some reason, so I'll just tag you here

@cgillum @ConnorMcMahon

@glennamanns glennamanns changed the base branch from master to dev October 23, 2018 18:49
Copy link
Member

@cgillum cgillum left a comment

Choose a reason for hiding this comment

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

These changes look good to me, though I don't fully understand how the tests are being run. :) A couple questions that would be helpful for me as a rookie reviewer:

  • Do these tests execute any binding logic, or just the ADAL calls?
  • How are the tests actually being run? Said another way, what does TestHelpers.RunTestAsync<T> do?

INameResolver nameResolver = GetValidSettingsForTests();
IAadClient aadClient = new AadClient(nameResolver);

var methodInfo = typeof(RealTokenFunctions).GetMethod("ClientCredentials");
Copy link
Member

Choose a reason for hiding this comment

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

methodInfo is not used here or in several tests below. Can these lines be deleted?

@glennamanns
Copy link
Contributor Author

@cgillum
RunTestAsync spawns up a new JobHost using a mocked EasyAuthClient, AadClient, and NameResolver. In order to get the client ID & secret for doing a "real" client credentials, I added a new appsettings.json and passed in a real NameResolver to RunTestAsync, rather than a fake (mocked) one.

This JobHost then calls whatever method was passed to RunTestAsync. In the three tests I added, the method is ClientCredentials (in the RealTokenFunctions class). That function has a Token binding, which follows the normal client credentials flow to get a real token.

Similar to the discussion we had yesterday about the Graph binding tests, appsettings.json needs four things: a client ID, client secret, open ID issuer, and hostname. To avoid checking in secrets, I left all of these blank. Since we want these to run as part of CI, I'm not really sure where to store/retrieve the secret.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants