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

[full-ci] add percent to usernames #39242

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

butonic
Copy link
Member

@butonic butonic commented Sep 15, 2021

we ran into this when trying a user that had an email as the username. ocis-web would encode the @ as %40 and the chi router would not match the route. See go-chi/chi#659

Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
@update-docs
Copy link

update-docs bot commented Sep 15, 2021

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

Copy link
Contributor

@phil-davis phil-davis left a comment

Choose a reason for hiding this comment

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

There should be matching scenarios in of the apiProvisioning-v1/v2 suites.
One change has been made in apiProvisioning-v1 and 3 in apiProvisioning-v2.
Maybe there could/should be 4 changes in each?

@phil-davis phil-davis self-requested a review September 15, 2021 15:18
@sonarcloud
Copy link

sonarcloud bot commented Sep 15, 2021

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 0 Code Smells

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

@ownclouders
Copy link
Contributor

💥 Acceptance tests pipeline apiProvisioning-v1-mariadb10.2-php7.4 failed. The build has been cancelled.

https://drone.owncloud.com/owncloud/core/32426/53/1

@phil-davis phil-davis changed the title add percent to usernames [full-ci] add percent to usernames Sep 16, 2021
@phil-davis
Copy link
Contributor

The % sign is not allowed in a username in oC10:

{"reqId":"54sWVWbrPSx9rqEg7rqp","level":3,"time":"2021-09-16T02:14:36+00:00","remoteAddr":"192.168.1.64","user":"admin","app":"ocs_api","method":"POST","url":"\/ocs\/v2.php\/cloud\/users","message":"Failed addUser attempt with exception: Only the following characters are allowed in a username: \"a-z\", \"A-Z\", \"0-9\", and \"+_.@-'\""}

So we know that the test cases in the first commit will not pass.

I put the test cases for % in a username into separate examples tables that are skipped ownCloud 10. That will let the CI pass here.
But I am not sure what the actual requirement is. Are actual literal % signs allowed in "username" strings in oCIS? Or do we want to test something different?

@phil-davis
Copy link
Contributor

There was an unrelated fail - see issue #39244

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants