Proposal: reusing context basic auth result in git http implementation

[beeonthego]

Current basic auth flow - is it a password? is it a password again? is it a token?

In the current implementation, when cloning/committing private repo over http/https using basic auth header, for each request, Gitea tries the following workflow

1. treating basic auth token as password and attempting to sign in user when creating context in middleware (modules/context/context.go).

2. treating basic auth token as password and attempting to sign in user AGAIN in routers/repo/http.go

3. validating it as token and verifying access to repo.

Each sign in operation involves hashing provided token and comparing it with the record retrieved from db. These are two wasted cycles in each request to perform git operations on private repo over http/https.

The proposed basic auth workflow - is it a token? is it a password?

If basic auth header is present, in context middleware, we will first attempt to validate the header as application token, then treat it as password. we can set a flag in ctx.Data to indicate whether it is plain text password. The decision can be made in routers whether to accept/reject auth by plain text password using the current logic.

If a valid token is provided (for example when Drone clones a private repo using token from a netrc file), then there will only be one validation cycle needed.

The reason behind is that there are potentially more git operations over http than login requests from users.

In order to keep existing integration with Drone, we will look for token both in username and password, using the same logic as it is done now.

Are there any concerns to this approach?

[techknowlogick]

I'm in favour of removing duplicate DB calls, however I think that the concern of token vs basic in regards to performance of hashing can be dismissed due to the other ticket of potentialy hashing tokens too. Please let me know if I understood your proposal correctly as I am in a timezone that is ~10hours different from mine and may be a touch jetlagged.

[beeonthego]

@techknowlogick Thanks for the prompt response. I was not referring to hashing token. I meant a token in basic auth header should first be treated as token instead of a password. It is far more common to have a token in basic auth header in daily operations. Taking the default Drone integration for example, a user may log into to Drone once in a while using Gitea password, but every build will trigger a cloning request using access token from netrc file.

The current basic auth flow is barking at the wrong tree when Drone provides a valid token for cloning private repo. It assumes the token is a password, tries to sign in user and fails. Then it tries to sign in user again and fails again. Then it finally gets smarter, treating a token as a token, and succeeds. This happens to every valid cloning request, no matter token is hashed or not.

I am proposing to validate basic auth header content as token first, then validate it as a password if it is not a token. We can do this in one go in context module, save the result in context, and apply the accept/reject logic in routers. what do you think?