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

add claims and regex policy selector #2248

Merged
merged 11 commits into from
Jul 23, 2021
Merged

add claims and regex policy selector #2248

merged 11 commits into from
Jul 23, 2021

Conversation

butonic
Copy link
Member

@butonic butonic commented Jul 1, 2021

Using the proxy config file, it is now possible to let let the IdP determine the routing policy by sending an ocis.routing.policy claim or by matching username, email or id using regexes.

  • tests
  • update changelog
  • update docs

This PR can stand on its own. But the proxy and the docs need an overhaul in subsequent PRs:

  • Configuring the policy selector is only possible via the config file
  • The proxy should always use the cs3 api. get rid of configuration choices here
  • We should use the cs3 api to authenticate requests ... yes it would mean more latency when authenticating, but we can cache the token afterwards. It would remove the duplicate code in the proxy and in the auth providers. This should allow us to actually get rid of several middlewares in here.
  • when moving the oidc auth into reva we can do the claims mapping there, including custom porperties like ocis.routing.policy or ocis.id

@butonic butonic requested a review from C0rby July 1, 2021 21:27
@butonic butonic changed the title add claims policy selector add claims and regex policy selector Jul 2, 2021
@butonic butonic marked this pull request as draft July 2, 2021 06:55
@butonic butonic self-assigned this Jul 2, 2021
@butonic butonic added the Category:Enhancement Add new functionality label Jul 2, 2021
Copy link
Contributor

@C0rby C0rby left a comment

Choose a reason for hiding this comment

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

Unit tests for NewClaimsSelector and NewRegexSelector would be awesome.

@butonic butonic force-pushed the claims-policy-selector branch from a8ee1dd to dc71a60 Compare July 2, 2021 13:15
@butonic butonic marked this pull request as ready for review July 2, 2021 15:25
@butonic butonic requested review from refs and C0rby July 2, 2021 15:25
@butonic butonic force-pushed the claims-policy-selector branch from e1fe4dc to 47e0d4c Compare July 5, 2021 11:22
@ownclouders
Copy link
Contributor

💥 Acceptance tests Core-API-Tests-owncloud-storage-6 failed. The build is cancelled...

@ownclouders
Copy link
Contributor

💥 Acceptance tests Core-API-Tests-ocis-storage-10 failed. The build is cancelled...

3 similar comments
@ownclouders
Copy link
Contributor

💥 Acceptance tests Core-API-Tests-ocis-storage-10 failed. The build is cancelled...

@ownclouders
Copy link
Contributor

💥 Acceptance tests Core-API-Tests-ocis-storage-10 failed. The build is cancelled...

@ownclouders
Copy link
Contributor

💥 Acceptance tests Core-API-Tests-ocis-storage-10 failed. The build is cancelled...

@ownclouders
Copy link
Contributor

💥 Acceptance tests Core-API-Tests-ocis-storage-9 failed. The build is cancelled...

@butonic butonic force-pushed the claims-policy-selector branch from 959d635 to e57c93d Compare July 6, 2021 21:27
@ownclouders
Copy link
Contributor

💥 Acceptance tests Core-API-Tests-ocis-storage-10 failed. The build is cancelled...

@butonic butonic force-pushed the claims-policy-selector branch from e57c93d to d1d0861 Compare July 8, 2021 22:01
@ownclouders
Copy link
Contributor

💥 Acceptance tests Core-API-Tests-ocis-storage-10 failed. The build is cancelled...

@ownclouders
Copy link
Contributor

💥 Acceptance tests Core-API-Tests-ocis-storage-9 failed. The build is cancelled...

@ownclouders
Copy link
Contributor

💥 Acceptance tests Core-API-Tests-ocis-storage-10 failed. The build is cancelled...

@ownclouders
Copy link
Contributor

💥 Acceptance tests Core-API-Tests-ocis-storage-9 failed. The build is cancelled...

1 similar comment
@ownclouders
Copy link
Contributor

💥 Acceptance tests Core-API-Tests-ocis-storage-9 failed. The build is cancelled...

@ownclouders
Copy link
Contributor

💥 Acceptance tests Core-API-Tests-ocis-storage-10 failed. The build is cancelled...

2 similar comments
@ownclouders
Copy link
Contributor

💥 Acceptance tests Core-API-Tests-ocis-storage-10 failed. The build is cancelled...

@ownclouders
Copy link
Contributor

💥 Acceptance tests Core-API-Tests-ocis-storage-10 failed. The build is cancelled...

@ownclouders
Copy link
Contributor

💥 Acceptance tests Core-API-Tests-owncloud-storage-10 failed. The build is cancelled...

@ownclouders
Copy link
Contributor

💥 Acceptance tests Core-API-Tests-ocis-storage-9 failed. The build is cancelled...

@ownclouders
Copy link
Contributor

💥 Acceptance tests Core-API-Tests-ocis-storage-10 failed. The build is cancelled...

@butonic butonic force-pushed the claims-policy-selector branch from 0be22de to 030af7b Compare July 19, 2021 13:49
@ownclouders
Copy link
Contributor

💥 Acceptance tests Core-API-Tests-owncloud-storage-10 failed. The build is cancelled...

@wkloucek wkloucek mentioned this pull request Jul 19, 2021
9 tasks
@butonic
Copy link
Member Author

butonic commented Jul 20, 2021

@butonic
Copy link
Member Author

butonic commented Jul 20, 2021

  • we need a cookie to switch users between routes because some requests, especially config.json are sent without a Bearer header. To route them properly we have to fall back to a cookie.

@butonic
Copy link
Member Author

butonic commented Jul 20, 2021

[tusd] 2021/07/19 14:03:15 event="ChunkWriteComplete" id="eadee146-5057-4433-861e-e7eebdf387db" bytesWritten="7" 
[tusd] 2021/07/19 14:03:15 event="ResponseOutgoing" status="204" method="PATCH" path="/eadee146-5057-4433-861e-e7eebdf387db" requestId="b518997e-976c-4db1-8512-4fc0294c4a51" 
[tusd] 2021/07/19 14:03:15 event="RequestIncoming" method="PATCH" path="/eadee146-5057-4433-861e-e7eebdf387db" requestId="40174af3-b638-4a58-9b77-fab7efb86d4a" 
[tusd] 2021/07/19 14:03:15 event="ResponseOutgoing" status="404" method="PATCH" path="/eadee146-5057-4433-861e-e7eebdf387db" error="upload not found" requestId="40174af3-b638-4a58-9b77-fab7efb86d4a" 
2021/07/19 14:03:15 httputil: ReverseProxy read error during body copy: unexpected EOF
{"level":"error","service":"ocs","error":"{\"code\":404,\"detail\":\"The requested user could not be found\",\"status\":\"Not Found\"}","userid":"Alice","time":"2021-07-19T14:03:16Z","message":"could not get account for user"}
{"level":"error","service":"proxy","error":"{\"id\":\"com.owncloud.api.accounts\",\"code\":401,\"detail\":\"account not found or invalid credentials\",\"status\":\"Unauthorized\"}","query":"login eq 'Alice' and password eq '123456'","time":"2021-07-19T14:03:16Z","message":"error fetching from accounts-service"}
[tusd] 2021/07/19 14:03:16 event="RequestIncoming" method="PATCH" path="/cd44bc26-bb11-48db-b1fa-4749d36a4a60" requestId="b4f8e24a-37b1-44fd-bfd6-178911370024" 
[tusd] 2021/07/19 14:03:16 event="ChunkWriteStart" id="cd44bc26-bb11-48db-b1fa-4749d36a4a60" maxSize="10" offset="0" 

🤔

@butonic
Copy link
Member Author

butonic commented Jul 20, 2021

I found cs3org/reva#1904

Does this PR change the request flow and cause the datagateway to spew errors?!? how?

@wkloucek
Copy link
Contributor

@butonic I added the cookie logic to this PR

@butonic
Copy link
Member Author

butonic commented Jul 21, 2021

Custom claims mapping in #2315

butonic and others added 9 commits July 23, 2021 08:07
Signed-off-by: Jörn Friedrich Dreyer <[email protected]>
Signed-off-by: Jörn Friedrich Dreyer <[email protected]>
Signed-off-by: Jörn Friedrich Dreyer <[email protected]>
Signed-off-by: Jörn Friedrich Dreyer <[email protected]>
Signed-off-by: Jörn Friedrich Dreyer <[email protected]>
Signed-off-by: Jörn Friedrich Dreyer <[email protected]>
Signed-off-by: Jörn Friedrich Dreyer <[email protected]>
@butonic butonic force-pushed the claims-policy-selector branch from 5fd2756 to bea986f Compare July 23, 2021 08:55
Signed-off-by: Jörn Friedrich Dreyer <[email protected]>
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

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

58.8% 58.8% Coverage
0.0% 0.0% Duplication

Signed-off-by: Jörn Friedrich Dreyer <[email protected]>
@butonic butonic merged commit dcd8327 into master Jul 23, 2021
@delete-merged-branch delete-merged-branch bot deleted the claims-policy-selector branch July 23, 2021 11:45
ownclouders pushed a commit that referenced this pull request Jul 23, 2021
Merge: 1e4f833 07bb4a1
Author: Jörn Friedrich Dreyer <[email protected]>
Date:   Fri Jul 23 13:45:43 2021 +0200

    Merge pull request #2248 from owncloud/claims-policy-selector

    add claims and regex policy selector
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category:Enhancement Add new functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants