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

IDP: allow using cs3 api to authenticate users #3825

Merged
merged 5 commits into from
May 20, 2022
Merged

IDP: allow using cs3 api to authenticate users #3825

merged 5 commits into from
May 20, 2022

Conversation

butonic
Copy link
Member

@butonic butonic commented May 18, 2022 •
edited
Loading

The IDP now directly authenticates users using the CS3 API instead of LDAP.

@butonic butonic force-pushed the lico-cs3-update branch 4 times, most recently from b2349a3 to 74ac7bb Compare May 18, 2022 14:09
@butonic
IDP: directly use CS3 API to authenticate users
78950ae 
Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
@butonic butonic marked this pull request as ready for review May 18, 2022 14:32
@butonic butonic requested review from rhafer and micbar May 18, 2022 14:32
@rhafer
Copy link
Contributor

rhafer commented May 18, 2022

Hm, I am a bit surprised that you want to make this the default at the current point in time. This seems to be a bit risky to me.
I thought this was only meant as an option (e.g. for the migration case, when using the owncloudsql cs3 backend). We could still switch defaults at a later point (after we gained some experience and had some chance to stabilized this)

@mmattel
Copy link
Contributor

mmattel commented May 18, 2022 •
edited
Loading

Does this impact any documentation relevant changes?

@butonic
keep ldap as default
08b80c2 
Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
@butonic
update docs and changelog
1a872c8 
Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
@butonic
Copy link
Member Author

butonic commented May 19, 2022

@rhafer changed back to ldap backend by default
@mmattel added a few desc: for the idp, no relevant changes as we stay with ldap for now. I'll submut a PR to update the docs for a parallel deployment.

@butonic butonic self-assigned this May 19, 2022
@butonic butonic changed the title IDP: directly use cs3 api to authenticate users IDP: allow using cs3 api to authenticate users May 19, 2022
kobergj
kobergj requested changes May 19, 2022
View reviewed changes
changelog/unreleased/idp-directly-use-cs3-to-authenticate.md Outdated Show resolved Hide resolved
extensions/idp/pkg/backends/cs3/bootstrap/cs3.go Show resolved Hide resolved
extensions/idp/pkg/backends/cs3/bootstrap/cs3.go Outdated Show resolved Hide resolved
extensions/idp/pkg/backends/cs3/identifier/cs3.go Outdated Show resolved Hide resolved
extensions/idp/pkg/backends/cs3/identifier/cs3.go Outdated Show resolved Hide resolved
extensions/idp/pkg/backends/cs3/identifier/cs3.go Outdated Show resolved Hide resolved
extensions/idp/pkg/service/v0/service.go Outdated Show resolved Hide resolved
go.mod Show resolved Hide resolved
rhafer
rhafer requested changes May 20, 2022
View reviewed changes
extensions/idp/pkg/backends/cs3/identifier/cs3.go Outdated Show resolved Hide resolved
extensions/idp/pkg/backends/cs3/identifier/cs3.go Outdated Show resolved Hide resolved
extensions/idp/pkg/backends/cs3/identifier/user.go Outdated Show resolved Hide resolved
@butonic
return errors, clean up
d56bbd6 
Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
@butonic butonic requested review from rhafer and kobergj May 20, 2022 10:33
kobergj
kobergj requested changes May 20, 2022
View reviewed changes
Copy link
Collaborator

@kobergj kobergj left a comment

Choose a reason for hiding this comment

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

As I stated before: You can't trick me 😜

extensions/idp/pkg/backends/cs3/identifier/cs3.go Show resolved Hide resolved
extensions/idp/pkg/backends/cs3/identifier/session.go Show resolved Hide resolved
extensions/idp/pkg/backends/cs3/identifier/user.go Show resolved Hide resolved
extensions/idp/pkg/backends/cs3/identifier/user.go Show resolved Hide resolved
extensions/idp/pkg/backends/cs3/identifier/user.go Show resolved Hide resolved
extensions/idp/pkg/backends/cs3/identifier/user.go Show resolved Hide resolved
extensions/idp/pkg/backends/cs3/identifier/user.go Show resolved Hide resolved
extensions/idp/pkg/backends/cs3/identifier/user.go Show resolved Hide resolved
extensions/idp/pkg/backends/cs3/identifier/user.go Show resolved Hide resolved
extensions/idp/pkg/backends/cs3/identifier/user.go Show resolved Hide resolved
@ownclouders
Copy link
Contributor

💥 Acceptance test Core-API-Tests-ocis-storage-7 failed. Further test are cancelled...

@butonic
add missing godoc to make hound vJulian happy
9ac13e6 
Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
@sonarcloud
Copy link

sonarcloud bot commented May 20, 2022

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 3 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

rhafer
rhafer approved these changes May 20, 2022
View reviewed changes
Copy link
Contributor

@rhafer rhafer left a comment

Choose a reason for hiding this comment

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

looks good to me now. You might want to wait for HoundCi @kobergj to check again ...

@butonic butonic merged commit a7f9e14 into master May 20, 2022
@delete-merged-branch delete-merged-branch bot deleted the lico-cs3-update branch May 20, 2022 12:24
ownclouders pushed a commit that referenced this pull request May 20, 2022
@butonic
commit a7f9e14
1788e7d 
Merge: a0f5c7a 9ac13e6
Author: Jörn Friedrich Dreyer <jfd@owncloud.com>
Date:   Fri May 20 12:24:06 2022 +0000

    Merge pull request #3825 from owncloud/lico-cs3-update

    IDP: allow using cs3 api to authenticate users
@rhafer rhafer mentioned this pull request May 24, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mmattel mmattel mmattel left review comments

@rhafer rhafer rhafer approved these changes

@kobergj kobergj kobergj approved these changes

@micbar micbar Awaiting requested review from micbar

Assignees

@butonic butonic

Labels
None yet
Projects
Infinite Scale Team Board
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@butonic @rhafer @mmattel @ownclouders @kobergj