test: add full OIDC fake IDP #9317
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Begin work on unit testing refactor
johnstcn
reviewed
Aug 25, 2023
| "access_token": f.newToken(email), | ||
| "refresh_token": refreshToken, | ||
| "token_type": "Bearer", | ||
| "expires_in": int64((time.Minute * 5).Seconds()), |
There was a problem hiding this comment.
Follow-up: this could be a useful config knob down the line.
There was a problem hiding this comment.
Agreed. We can add more knobs as we make more unit tests. At the moment, the time stuff can be a bit tricky, so instead I just made that ExpireOauthToken which just changes the expiry to the past.
It was easier than trying to sync some clock manipulation in the various places we need.
johnstcn
approved these changes
Aug 25, 2023
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What this does
This adds the new fake OIDC provider and refactors existing tests, however we should still write more tests!
This creates a realistic IDP for OIDC testing. It handles all the endpoints expected of an IDP, and enforces the codes/states/tokens it returns. This was implemented because our OIDC logic is growing in complexity, and we need a proper IDP to run tests against. The existing mock was very relaxed, and did not support some features like refreshing.
Our old fake was lacking a few areas:
oauth2.HTTPClientcontext value for proper auth in some cases (PKI)The new fake is essentially an IDP that we can continue to expand to cover more cases and more regressions.
Existing unit tests
All existing OIDC unit tests were moved to the new fake.
Future Work
More tests
We should write more tests around our oauth httpmw handler and api key extract. We can probably do it without starting a full Coderd and getting much faster tests.
I think our current tests do not test all of the possible edge cases and combinations.
Quicker Tests
The new IDP supports a complete in memory HTTP client that never makes HTTP calls. This makes things quicker and easier to debug. Unfortunately making
coderdtest.Newuse this HTTP client was non-trivial without refactoring that. To avoid touching more things, I am using theoidctest.WithServing()to spin up ahttptest.Serverwhich handles real network requests.Make refresh tokens sync settings
I discovered in doing this that our group and role syncs do not happen on oauth refresh tokens. This should be remedied. Unit tests that check this have been skipped.