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

Over-zealous username validation in UI #4150

Open
2 of 7 tasks
mrichar1 opened this issue Jun 6, 2018 · 9 comments
Open
2 of 7 tasks

Over-zealous username validation in UI #4150

mrichar1 opened this issue Jun 6, 2018 · 9 comments
Labels
topic/authentication type/proposal The new feature has not been accepted yet but needs to be discussed first.

Comments

@mrichar1
Copy link
Contributor

mrichar1 commented Jun 6, 2018

  • Gitea version (or commit ref): 1.4.1
  • Git version: 1.8.3.1
  • Operating system: Redhat EL7.4
  • Database (use [x]):
    • PostgreSQL
    • MySQL
    • MSSQL
    • SQLite
  • Can you reproduce the bug at https://try.gitea.io:
    • Yes (provide example URL)
    • No
    • Not relevant
  • Log gist:

Description

We use kerberos via apache to authenticate hosts to gitea. Hosts have kerberos principals of the form git/host.example.com@EXAMPLE.COM which get translated to the username git/host.example.com

When we set ENABLE_REVERSE_PROXY_AUTO_REGISTRATION to true then these accounts are correctly created in gitea when the host runs git commands against our server. However since the account is 'new' it is not in the appropriate teams, so the first run fails, and we then have to set up team membership etc and re-run the processes.

To fix this I'm trying to pre-create these accounts using the UI - however when I try to create a new user with name git/host.example.com I receive the error Username must be valid alphanumeric, dash(-_) or dot characters.

Since auto-generated usernames with these characters seem to work fine, would it be possible to alter this validation to allow a broader range of usernames, filtering only those which would make gitea actually fail?

@lafriks
Copy link
Member

lafriks commented Jun 6, 2018

/ can not be allowed as that could break functionality and even rise security issues

@mrichar1
Copy link
Contributor Author

mrichar1 commented Jun 6, 2018

gitea currently allows / in usernames through auto-creation of accounts, and I have several accounts which seem to function correctly with such names. I guess this means that the auto-creation code doesn't follow the same codepath as the UI/API use?

I'm interested to know what security issues might arise from the username containing unexpected characters... I'd hope that all such fields were only ever used with appropriate escaping, with parameterized queries etc to avoid sql injection and similar issues.

I'd obviously be keen to keep / in usernames, since this is useful functionality to us - and I'd argue that it would be better to relax the validation and instead test what actually happens with extra characters.
That way the code can be improved to handle these cases, instead of arbitrarily limiting them because of the unknown effects they might have (especially since, as we can see here, there are unexpected routes to them existing!)

@gszy
Copy link

gszy commented Jul 29, 2018

What’s your opinion on allowing @ in usernames?

@lafriks
Copy link
Member

lafriks commented Jul 29, 2018

Also quite risky to break something. Would all git clients support such urls?

@gszy
Copy link

gszy commented Jul 29, 2018

TL; DR: don’t know…

git check-ref-format --branch 'a@a' returns 0, but man git-push doesn’t explain what characters are allowed in path/to/repo (in ssh://[user@]host.xz[:port]/path/to/repo.git/ example). Neither GitHub nor GitLab allow @ in usernames or project names.

@ is commonly used to mention a user or to refer to specific commit. Unless username’s first character is @ or it looks like me@something­‑that­‑looks­‑like­‑commit­‑SHA, that shouldn’t be a very big problem, though it would be if we could refer to branches, like me@master (could be solved with a config option).

@lafriks lafriks added the type/proposal The new feature has not been accepted yet but needs to be discussed first. label Jul 29, 2018
@lafriks
Copy link
Member

lafriks commented Jul 29, 2018

I do think that we should stick to current behaviour to not break things and keep compatibility with future changes (especially if we implement projects/groups under organizations).

@cwchristerw
Copy link

cwchristerw commented Aug 16, 2020

I need "-" to work in org name when transferring repo between orgs. When trying to transfer repository from "cwinfo-private" to "warengroup-private". UI will show error "The new owner name is not valid."

@silverwind
Copy link
Member

I think we should align our validation for org and repo name to a common denominator among forges like GitHub/GitLab. GitHub for example allow - as repo name, but not as a org name.

The most accurate description of what is allowed in repos is this:

it looks like github allows [A-Za-z0-9_.-], and transforms all other characters to "-".

For orgs:

The name may only contain alphanumeric characters or single hyphens, and cannot begin or end with a hyphen.

@silverwind
Copy link
Member

silverwind commented May 31, 2024

So maybe these, and they could be made configurable I suppose.

  • ^[A-Za-z0-9_.-]+$ for repos
  • ^(?![-])[A-Za-z0-9]+(?:[-][A-Za-z0-9]+)*?(?<!-)$ for orgs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic/authentication type/proposal The new feature has not been accepted yet but needs to be discussed first.
Projects
None yet
Development

No branches or pull requests

5 participants