owncloud / openidconnect

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

[QA] admin cannot login, when basic_auth_guest_only is true #261

Closed jnweiger closed 1 year ago

jnweiger commented 1 year ago

Seen while testing openidconnect 2.2.0-rc.6 with core 10.11.0

Expected behaviour:

DeepDiver1975 commented 1 year ago

Works as designed. Only guests 🤷

DeepDiver1975 commented 1 year ago

I suggest to revert https://github.com/owncloud/openidconnect/pull/253 and get this release rolling.

We have customers waiting for other features of this release.

@jvillafanez @hodyroff @jnweiger objections

jvillafanez commented 1 year ago

https://github.com/owncloud/openidconnect/pull/265 would fix this, but we need to decide what to do with the feature as a whole

DeepDiver1975 commented 1 year ago

Absolutly

DeepDiver1975 commented 1 year ago

revert pr created ...... so that we are prepares once the decision is there ..... https://github.com/owncloud/openidconnect/pull/268

jnweiger commented 1 year ago

I agree with the revert and move everything to work with groups. #265 could carry guests and admin group to fix my case. The naming of the key in #265 is weird then and should be simplified.

jnweiger commented 1 year ago

@DeepDiver1975 I cannot confirm that the code is working with 2.2.0-rc.7

With 10.11.0 and openidconnect-2.2.0-rc.7 installed:

/var/www/owncloud# find . -name \*.php | xargs grep basic_auth_guest_only
/var/www/owncloud#

With daily master and openidconnect-2.2.0-rc.7 installed:

/var/www/owncloud# find . -name \*.php | xargs grep basic_auth_guest_only
/var/www/owncloud#

/var/www/owncloud# find . -name *.php | xargs grep basic_auth_guest_only ./config/config.apps.sample.php: * Possible keys: openid-connect.basic_auth_guest_only BOOL ./config/config.apps.sample.php:'openid-connect.basic_auth_guest_only' => false, ./config/config.php: 'openid-connect.basic_auth_guest_only' => true, /var/www/owncloud#

DeepDiver1975 commented 1 year ago

We removed this feature. It will be implemented in core. Out of scope