owncloud / openidconnect

OpenId Connect (OIDC) Integration for ownCloud
GNU General Public License v2.0
6 stars 2 forks source link

Read openid configuration from DB first before using config.php #197

Closed stijnbrouwers closed 2 years ago

stijnbrouwers commented 2 years ago

Description

Auto-provision is not working when setting the configuration in the database. It only works when you change config.php. This change is in line with the default behavior for saving OpenID settings (https://doc.owncloud.com/server/next/admin_manual/configuration/user/oidc/oidc.html#save-settings-in-the-database).

Related Issue

Motivation and Context

Settings the auto-provision using the proposed method (in database) was not working without this change

How Has This Been Tested?

Log in with a user that not exists resulted in an error (user xxx is not known.) Applying the changes in this pull request created the user in owncloud.

Types of changes

Checklist:

Open tasks:

CLAassistant commented 2 years ago

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


Stijn Brouwers seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

sonarcloud[bot] commented 2 years ago

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 1 Code Smell

22.2% 22.2% Coverage
0.0% 0.0% Duplication

mmattel commented 2 years ago

My approval was accidentially, pls do not count on it.

DeepDiver1975 commented 2 years ago

That change is quite reasonable - thanks for bringing this to our attention!

I'd like to avoid the code duplication. So it might be reasonable to inject Client into the AutoProvisioning server and call getOpenIdConfig - refs https://github.com/owncloud/openidconnect/blob/86c11a575e8f6e5d9f63436141cd45ce6cbc6b76/lib/Client.php#L96

stijnbrouwers commented 2 years ago

@DeepDiver1975 totally agree the change is applied.

DeepDiver1975 commented 2 years ago

And please have a look at the https://github.com/owncloud/openidconnect/pull/197#issuecomment-999593522 - THX a lot

stijnbrouwers commented 2 years ago

@DeepDiver1975 The comment you are refering to is the CLA right? Is there a nightly processing or something? I agreed to it but it remains "not signed yet" CLA

stijnbrouwers commented 2 years ago

@DeepDiver1975 I am developing from a new machine and there was a typo in my git.config email. This prevents the CLA to proceed and it's not recommended by Github to change the email of previous commits. Therefore, I suggest we close this PR and I will open a new one (#198 )