owncloud / ocis

:atom_symbol: ownCloud Infinite Scale Stack
https://doc.owncloud.com/ocis/next/
Apache License 2.0
1.38k stars 181 forks source link

Usernames containing + cannot be autoprovisioned #4078

Closed cakiwi closed 2 years ago

cakiwi commented 2 years ago

Describe the bug

Usernames containing + cannot be autoprovisioned

Steps to reproduce

Steps to reproduce the behavior:

  1. Run a deployment using keycloak.
  2. Set realm login to use email as username (not required, can just set the username to contain +)
  3. Register a user using + in the local part. a+b@example.org
  4. Sign-in to oCIS

Expected behavior

Login to oCIS should occur.

Actual behavior

Login Error. Your user session is invalid or has expired. If you like to login with a different user please proceed to exit.

{"level":"error","service":"proxy","error":"500 Internal Server Error","time":"2022-07-01T16:59:23Z","message":"Error creating user"}
{"level":"error","service":"proxy","error":"500 Internal Server Error","time":"2022-07-01T16:59:23Z","message":"Autoprovisioning user failed"}
{"level":"error","service":"proxy","error":"500 Internal Server Error","time":"2022-07-01T16:59:23Z","message":"Could not get user by claim"}
{"level":"error","service":"proxy","error":"500 Internal Server Error","time":"2022-07-01T16:59:23Z","message":"Error creating user"}
{"level":"error","service":"proxy","error":"500 Internal Server Error","time":"2022-07-01T16:59:23Z","message":"Autoprovisioning user failed"}
{"level":"error","service":"proxy","error":"500 Internal Server Error","time":"2022-07-01T16:59:23Z","message":"Could not get user by claim"}

Setup

owncloud/ocis:2.0.0-beta.4

https://owncloud.dev/ocis/deployment/ocis_keycloak/ Originally changed realm login to use email as username.

C0rby commented 2 years ago

Just some notes because I can't work on this for the next two weeks.

So the issue is here: https://github.com/cs3org/reva/blob/0b65112600ccfd07504c8927d96c7a1e9ac85e7b/pkg/utils/ldap/identity.go#L534

We aren't escaping the memberName correctly which results in this error when parsing the filter: LDAP Result Code 201 "Filter Compile Error": ldap: invalid characters for escape in filter: encoding/hex: invalid byte: U+002B '+' (&(objectclass=groupOfNames)(member=uid=corby\+,ou=users,o=libregraph-idm))

Our filter looks like this (&(objectclass=groupOfNames)(member=uid=corby\\5c+,ou=users,o=libregraph-idm)) but the parser expects (&(objectclass=groupOfNames)(member=uid=corby\\2B,ou=users,o=libregraph-idm))"

My temporary change to check the fix was this:

func (i *Identity) getGroupMemberFilter(memberName string) string {
      return fmt.Sprintf("(&%s(objectclass=%s)(%s=%s))",
          i.Group.Filter,
          i.Group.Objectclass,
          i.Group.Schema.Member,
~         strings.ReplaceAll(memberName, "+", "2B"),
      )
  }

With that change logging into oCIS was possible.

rhafer commented 2 years ago

Our filter looks like this (&(objectclass=groupOfNames)(member=uid=corby\\5c+,ou=users,o=libregraph-idm))

Actually I think that filter is correct. However, it seems there is a but in libregraph/idm that fails to correctly reescape the filter when decompiling it from BER to string. And the later for what ever reason tries to re-compile that string to BER, which fails because of wrong escaping :man_facepalming:

Somewhere here: https://github.com/libregraph/idm/blob/491c539a42bb90395ea23dcc11181891f244f2b4/pkg/ldapserver/search.go#L25

I think the is mainly caused by idm using it's own fliter compliation/de-compliation code instead of the stuff coming from go-ldap.

rhafer commented 2 years ago

Should be fixed with: https://github.com/libregraph/idm/pull/68

rhafer commented 2 years ago

Fix has been merged into ocis with: #4132

rhafer commented 2 years ago

Actually it still doesn't fully work. :sob: let's reopen...

rhafer commented 2 years ago

final fix was in idm https://github.com/libregraph/idm/pull/71 which was merged with #4200