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

Enable dynamic client registration #813

Merged
merged 1 commit into from
Nov 20, 2020
Merged

Enable dynamic client registration #813

merged 1 commit into from
Nov 20, 2020

Conversation

DeepDiver1975
Copy link
Member

@DeepDiver1975 DeepDiver1975 commented Nov 6, 2020 •
edited
Loading

Fixes #811

Screenshot from 2020-11-06 11-35-55

@update-docs

This comment has been minimized.

Sign in to view
@DeepDiver1975 DeepDiver1975 requested a review from C0rby November 6, 2020 10:33
@DeepDiver1975 DeepDiver1975 self-assigned this Nov 6, 2020
@DeepDiver1975 DeepDiver1975 requested a review from butonic November 6, 2020 10:35
IljaN
IljaN suggested changes Nov 6, 2020
View reviewed changes
Copy link
Contributor

@IljaN IljaN left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Set the value as default here instead if it should be the default:

ocis/konnectd/pkg/flagset/flagset.go

Line 305 in 8bef86c

EnvVars: []string{"KONNECTD_ALLOW_DYNAMIC_CLIENT_REGISTRATION"},

If not then this is only a configuration issue and this can be closed?

@DeepDiver1975
Copy link
Member Author

So this can be enabled using the env var? fine by me

How to set this env var for https://ocis-bridge.owncloud.works/ ?
And possibly other installations?

We might want this to be enabled by default ...

... but this is not my call ...

butonic
butonic approved these changes Nov 12, 2020
View reviewed changes
Copy link
Member

@butonic butonic left a comment

Choose a reason for hiding this comment

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

updated the PR to use the env var and default to true

@butonic
Copy link
Member

butonic commented Nov 12, 2020

yuck, the oidc spec really could use some better explanation of why you'd want this. Reading https://curity.io/resources/architect/openid-connect/openid-connect-understanding-dcr/ I think I finally understand the value. And yes, we should set KONNECTD_ALLOW_DYNAMIC_CLIENT_REGISTRATION to true in the code as the new default.

@butonic butonic requested a review from IljaN November 12, 2020 22:22
@felixboehm felixboehm added the OCIS-Fastlane Planned outside of the sprint label Nov 16, 2020
@DeepDiver1975 @kulmann
Fixes #811 - enable dynamic client registration
0859316 
Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
@kulmann
Copy link
Contributor

kulmann commented Nov 20, 2020

Rebased to current master

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@kulmann kulmann merged commit c362ff2 into master Nov 20, 2020
@delete-merged-branch delete-merged-branch bot deleted the bugfix/811 branch November 20, 2020 13:54
ownclouders pushed a commit that referenced this pull request Nov 20, 2020
@kulmann
commit c362ff2
64ea8c7 
Merge: f02d80d 0859316
Author: Benedikt Kulmann <benedikt@kulmann.biz>
Date:   Fri Nov 20 14:54:07 2020 +0100

    Merge pull request #813 from owncloud/bugfix/811

    Enable dynamic client registration
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@IljaN IljaN IljaN approved these changes

@butonic butonic butonic approved these changes

@C0rby C0rby Awaiting requested review from C0rby

Assignees

@DeepDiver1975 DeepDiver1975

Labels
OCIS-Fastlane Planned outside of the sprint
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Enable dynamic client registration on https://ocis-bridge.owncloud.works/
5 participants
@DeepDiver1975 @butonic @kulmann @IljaN @felixboehm