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.

Sign up for GitHub

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

Jump to bottom

port auth middleware "test client" for user tokens #12929

Merged
merged 10 commits into from
Feb 17, 2022
Merged

port auth middleware "test client" for user tokens #12929

merged 10 commits into from
Feb 17, 2022

Conversation

S11001001
Copy link
Contributor

@S11001001 S11001001 commented Feb 15, 2022 •
edited
Loading

Fixes #12881 by making the oauth2 test server invent user tokens instead of claim tokens if you ask it to. As with the claim tokens, these aren't "real" tokens backed by a participant server, so only a subset of the tests makes sense, and we abstract over the shared compatible tests accordingly.

Pull Request Checklist

  • Read and understand the contribution guidelines
  • Include appropriate tests
  • Set a descriptive title and thorough description
  • Add a reference to the issue this PR will solve, if appropriate
  • Include changelog additions in one or more commit message bodies between the CHANGELOG_BEGIN and CHANGELOG_END tags
  • Normal production system change, include purpose of change in description
  • If you mean to change the status of a component, please make sure you keep the Component Status page up to date.

NOTE: CI is not automatically run on non-members pull-requests for security
reasons. The reviewer will have to comment with /AzurePipelines run to
trigger the build.

@S11001001 S11001001 added team/ledger-clients Related to the Ledger Clients team's components. component/auth-middleware concern/testing labels Feb 15, 2022
@S11001001 S11001001 self-assigned this Feb 15, 2022
@S11001001
Copy link
Contributor Author

A little review of the components:

  1. the middleware server, com.daml.auth.middleware.oauth2.Server. Not touched here
  2. the oauth test server, com.daml.auth.oauth2.test.server.Server. Ported to produce user tokens in this PR
  3. the test client (which is also a server), which exercises both, com.daml.auth.oauth2.test.server.Client. Passes through tokens so not touched either
  4. ...test.server.Test, which tests by interacting with (3)

@S11001001 S11001001 marked this pull request as ready for review February 17, 2022 17:48
ray-roestenburg-da
ray-roestenburg-da approved these changes Feb 17, 2022
View reviewed changes
Copy link
Contributor

@ray-roestenburg-da ray-roestenburg-da left a comment

Choose a reason for hiding this comment

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

LGTM

@S11001001 S11001001 enabled auto-merge (squash) February 17, 2022 18:46
This was referenced Feb 17, 2022
Merged
Closed
@S11001001 S11001001 merged commit 3ae19d9 into main Feb 17, 2022
@S11001001 S11001001 deleted the 12881-oauth2-test-server-tests-user-tokens branch February 17, 2022 19:44
S11001001 added a commit that referenced this pull request Feb 22, 2022
@S11001001
test middleware user tokens with oauth2 server user tokens (#12991)
6bb438e 
TestMiddlewareUserToken now uses user tokens from the oauth
server as well, courtesy #12929. TestMiddlewareClaimsToken is the
only middleware-only (non-Client) test that switches the oauth
test server back to producing claims tokens, and contains all the
tests that depend on claims token semantics. These tests are in a
sense exercising the oauth server rather than the middleware.

The token returned by the oauth server is irrelevant for the behavior these
tests are exercising, so they are merely ported to always run on user tokens.

- CallbackUriOverride
- LimitedCallbackStore
- ClientLimitedCallbackStore
- ClientNoRedirectToLogin
- ClientYesRedirectToLogin
- ClientAutoRedirectToLogin

CHANGELOG_BEGIN
CHANGELOG_END
garyverhaegen-da added a commit that referenced this pull request Feb 22, 2022
@garyverhaegen-da
fix tf
7a61fce 
Goals:

- Reflect manual changes from #12996 in Terraform.
- Reflect manual changes from #12997 in Terraform.
- Update plugins to wirk with #12929.
- Keep running services working through the changes.

Details in commits.

CHANGELOG_BEGIN
CHANGELOG_END
@garyverhaegen-da garyverhaegen-da mentioned this pull request Feb 22, 2022
Merged
@S11001001 S11001001 mentioned this pull request Feb 22, 2022
Merged
7 tasks
S11001001 added a commit that referenced this pull request Feb 24, 2022
@S11001001
enable oauth2-test's user tokens in trigger service tests (#12994)
786906f 
Duplicates the trigger service's auth tests for the two modes of
the oauth test server from #12929, then selects out the
claim-token-dependent tests as explained in
#12831 (comment)

Part of #12831.

* also suppress claims-only tests in oracle-with-oauth2-test

* use inClaims for several tests

CHANGELOG_BEGIN
CHANGELOG_END
@S11001001 S11001001 mentioned this pull request Feb 24, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ray-roestenburg-da ray-roestenburg-da ray-roestenburg-da approved these changes

@realvictorprm realvictorprm Awaiting requested review from realvictorprm

@stefanobaghino-da stefanobaghino-da Awaiting requested review from stefanobaghino-da

Assignees

@S11001001 S11001001

Labels
component/auth-middleware concern/testing team/ledger-clients Related to the Ledger Clients team's components.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

port auth middleware "test client" and the tests that use it to user tokens
2 participants
@S11001001 @ray-roestenburg-da