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.

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

Usernames containing + cannot be autoprovisioned #4078

Closed
cakiwi opened this issue Jul 2, 2022 · 6 comments
Closed

Usernames containing + cannot be autoprovisioned #4078

cakiwi opened this issue Jul 2, 2022 · 6 comments
Assignees
Labels
Priority:p2-high Escalation, on top of current planning, release blocker Type:Bug

Comments

@cakiwi
Copy link

cakiwi commented Jul 2, 2022

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. [email protected]
  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.

@cakiwi cakiwi added the Type:Bug label Jul 2, 2022
@wkloucek wkloucek added the Priority:p2-high Escalation, on top of current planning, release blocker label Jul 4, 2022
@C0rby C0rby self-assigned this Jul 5, 2022
@C0rby
Copy link
Contributor

C0rby commented Jul 7, 2022

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 rhafer self-assigned this Jul 11, 2022
@rhafer
Copy link
Contributor

rhafer commented Jul 11, 2022

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 🤦‍♂️

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
Copy link
Contributor

rhafer commented Jul 12, 2022

Should be fixed with: libregraph/idm#68

@rhafer
Copy link
Contributor

rhafer commented Jul 13, 2022

Fix has been merged into ocis with: #4132

@rhafer rhafer closed this as completed Jul 13, 2022
@rhafer
Copy link
Contributor

rhafer commented Jul 13, 2022

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

@rhafer
Copy link
Contributor

rhafer commented Jul 14, 2022

final fix was in idm libregraph/idm#71 which was merged with #4200

@rhafer rhafer closed this as completed Jul 14, 2022
2403905 added a commit to 2403905/ocis that referenced this issue Jul 25, 2023
2403905 added a commit to 2403905/ocis that referenced this issue Jul 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority:p2-high Escalation, on top of current planning, release blocker Type:Bug
Projects
Archived in project
Development

No branches or pull requests

5 participants