we need use format as 'git clone https://oauth2:<token>@gitea.com/u… #10902

GQH1993 · 2020-03-31T09:12:03Z

…sername/testcode.git' to clone code ,and failure occurred in the process!

Please check the following:

Make sure you are targeting the master branch, pull requests on release branches are only allowed for bug fixes.
Read contributing guidelines: https://github.com/go-gitea/gitea/blob/master/CONTRIBUTING.md
Describe what your pull request does and which issue you're targeting (if any)

You MUST delete the content above including this line before posting, otherwise your pull request will be invalid.

…ername/testcode.git' to clone code ,and failure occurred in the process!

codecov-io · 2020-03-31T09:44:05Z

Codecov Report

Merging #10902 into master will increase coverage by 0.05%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master   #10902      +/-   ##
==========================================
+ Coverage   43.40%   43.45%   +0.05%     
==========================================
  Files         593      593              
  Lines       83343    83355      +12     
==========================================
+ Hits        36175    36225      +50     
+ Misses      42673    42641      -32     
+ Partials     4495     4489       -6

Impacted Files	Coverage Δ
routers/repo/http.go	`41.57% <100.00%> (+0.25%)`	⬆️
models/gpg_key.go	`53.42% <0.00%> (-0.53%)`	⬇️
models/repo_list.go	`77.32% <0.00%> (-0.18%)`	⬇️
routers/repo/view.go	`36.82% <0.00%> (+0.67%)`	⬆️
models/issue_milestone.go	`69.16% <0.00%> (+1.54%)`	⬆️
modules/process/manager.go	`78.31% <0.00%> (+3.61%)`	⬆️
routers/user/home.go	`58.27% <0.00%> (+4.09%)`	⬆️
modules/indexer/stats/db.go	`59.37% <0.00%> (+9.37%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bf847b9...36bd391. Read the comment docs.

zeripath · 2020-03-31T09:59:09Z

I don't understand how this could be helpful.

Let's take a look at the code:

gitea/routers/repo/http.go

Lines 170 to 247 in 73cf0e2

    
           // Check if username or password is a token 
        
           isUsernameToken := len(authPasswd) == 0 || authPasswd == "x-oauth-basic" 
        
           // Assume username is token 
        
           authToken := authUsername 
        
           if !isUsernameToken { 
        
           	// Assume password is token 
        
           	authToken = authPasswd 
        
           } 
        
           uid := sso.CheckOAuthAccessToken(authToken) 
        
           if uid != 0 { 
        
           	ctx.Data["IsApiToken"] = true 
        
           	authUser, err = models.GetUserByID(uid) 
        
           	if err != nil { 
        
           		ctx.ServerError("GetUserByID", err) 
        
           		return 
        
           	} 
        
           } 
        
           // Assume password is a token. 
        
           token, err := models.GetAccessTokenBySHA(authToken) 
        
           if err == nil { 
        
           	if isUsernameToken { 
        
           		authUser, err = models.GetUserByID(token.UID) 
        
           		if err != nil { 
        
           			ctx.ServerError("GetUserByID", err) 
        
           			return 
        
           		} 
        
           	} else { 
        
           		authUser, err = models.GetUserByName(authUsername) 
        
           		if err != nil { 
        
           			if models.IsErrUserNotExist(err) { 
        
           				ctx.HandleText(http.StatusUnauthorized, fmt.Sprintf("invalid credentials from %s", ctx.RemoteAddr())) 
        
           			} else { 
        
           				ctx.ServerError("GetUserByName", err) 
        
           			} 
        
           			return 
        
           		} 
        
           		if authUser.ID != token.UID { 
        
           			ctx.HandleText(http.StatusUnauthorized, fmt.Sprintf("invalid credentials from %s", ctx.RemoteAddr())) 
        
           			return 
        
           		} 
        
           	} 
        
           	token.UpdatedUnix = timeutil.TimeStampNow() 
        
           	if err = models.UpdateAccessToken(token); err != nil { 
        
           		ctx.ServerError("UpdateAccessToken", err) 
        
           	} 
        
           } else if !models.IsErrAccessTokenNotExist(err) && !models.IsErrAccessTokenEmpty(err) { 
        
           	log.Error("GetAccessTokenBySha: %v", err) 
        
           } 
        
           if authUser == nil { 
        
           	// Check username and password 
        
           	authUser, err = models.UserSignIn(authUsername, authPasswd) 
        
           	if err != nil { 
        
           		if models.IsErrUserProhibitLogin(err) { 
        
           			ctx.HandleText(http.StatusForbidden, "User is not permitted to login") 
        
           			return 
        
           		} else if !models.IsErrUserNotExist(err) { 
        
           			ctx.ServerError("UserSignIn error: %v", err) 
        
           			return 
        
           		} 
        
           	} 
        
           	if authUser == nil { 
        
           		ctx.HandleText(http.StatusUnauthorized, fmt.Sprintf("invalid credentials from %s", ctx.RemoteAddr())) 
        
           		return 
        
           	} 
        
           	_, err = models.GetTwoFactorByUID(authUser.ID) 
        
           	if err == nil { 
        
           		// TODO: This response should be changed to "invalid credentials" for security reasons once the expectation behind it (creating an app token to authenticate) is properly documented 
        
           		ctx.HandleText(http.StatusUnauthorized, "Users with two-factor authentication enabled cannot perform HTTP/HTTPS operations via plain username and password. Please create and use a personal access token on the user settings page") 
        
           		return 
        
           	} else if !models.IsErrTwoFactorNotEnrolled(err) { 
        
           		ctx.ServerError("IsErrTwoFactorNotEnrolled", err) 
        
           		return 
        
           	} 
        
           }

First of all, L171 checks if the password is empty or equals the special string "x-oauth-basic". If it is we set the flag isUsernameToken that the username should be checked if it is the oauth token. L173-L177 then set the potential authToken to either the username or password. Your change does not change this.

L178 checks if this authToken is an oauth token, returning a uid if so which is then used as the user.

L189-L218 allows local tokens to override oauth tokens but they're almost never going to collide.

At L220 if you have successfully looked up an oauth user at L178 authUser will not be nil - so these sections will not be used - if not your change makes no difference as isUsernameToken is not used here and there are no further references to isUsernameToken.

zeripath

This cannot be helpful. In this case the Username is not the Token the Password is the token.

GQH1993 · 2020-03-31T10:07:17Z

We use a different format  ,like gitlab use token clone code " git clone https://oauth2:[email protected]/vendor/pac...",   In order to be compatible with this way of use ;I recommend that when the username field is identified as 'oauth2', Determines that the current request is to use ACCESS TOKEN clone code

…

------------------ 原始邮件 ------------------ 发件人: "zeripath"<[email protected]>; 发送时间: 2020年3月31日(星期二) 晚上6:00 收件人: "go-gitea/gitea"<[email protected]>; 抄送: "像风一样飞翔"<[email protected]>;"Author"<[email protected]>; 主题: Re: [go-gitea/gitea] we need use format as 'git clone https://oauth2:<token>@gitea.com/u… (#10902) @zeripath requested changes on this pull request. This cannot be helpful. In this case the Username is not the Token the Password is the token. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe.

zeripath · 2020-03-31T10:10:10Z

It already does that. Your PR does nothing to change that.

zeripath · 2020-03-31T10:12:01Z

If the password is an oauth token the username can be absolutely anything AFAIC from that code.

GQH1993 · 2020-03-31T10:36:54Z

Thank you for your response ,and you can see  at len 171 if authPasswd mismatch condition ,isUsernameToken Will always be false;  at len 191 because isUsernameToken is false , token will not be used

…

------------------ 原始邮件 ------------------ 发件人: "zeripath"<[email protected]>; 发送时间: 2020年3月31日(星期二) 晚上6:12 收件人: "go-gitea/gitea"<[email protected]>; 抄送: "像风一样飞翔"<[email protected]>;"Author"<[email protected]>; 主题: Re: [go-gitea/gitea] we need use format as 'git clone https://oauth2:<token>@gitea.com/u… (#10902) If the password is an oauth token the username can be absolutely anything AFAIC from that code. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe.

GQH1993 · 2020-03-31T10:41:14Z

When  authUsername is 'oauth2' isUsernameToken will be changed to true. and token will be used;

…

------------------ 原始邮件 ------------------ 发件人: "zeripath"<[email protected]>; 发送时间: 2020年3月31日(星期二) 晚上6:12 收件人: "go-gitea/gitea"<[email protected]>; 抄送: "像风一样飞翔"<[email protected]>;"Author"<[email protected]>; 主题: Re: [go-gitea/gitea] we need use format as 'git clone https://oauth2:<token>@gitea.com/u… (#10902) If the password is an oauth token the username can be absolutely anything AFAIC from that code. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe.

GQH1993 · 2020-03-31T10:45:40Z

Thank you for your response ,and you can see at len 171 if authPasswd mismatch condition ,isUsernameToken Will always be false;
at len 191 because isUsernameToken is false , token will not be used

GQH1993 · 2020-03-31T10:45:49Z

When authUsername is 'oauth2' isUsernameToken will be changed to true. and token will be used;

zeripath · 2020-03-31T10:49:26Z

I suggest you take another look.

GQH1993 · 2020-03-31T11:06:43Z

I accept your opinion，but when we use 'https://oauth2:[email protected]/vendor/pac...' to clone code ，The system returns an exception in 198L, the query fails because the authUsername is 'oauth2' ; it will not continue to execute to 220L.

…ername/testcode.git' to clone code ,and failure occurred in the process!

GQH1993 · 2020-03-31T11:39:30Z

I have changed the modification method, please have a look,thank you~~

…ername/testcode.git' to clone code ,and failure occurred in the process!

zeripath · 2020-04-01T10:32:12Z

OK, So if I understand correctly - you're not talking about an external OAUTH2 token - but rather tokens that Gitea creates?

Here you're suggesting relaxing the constraint that Gitea's tokens require you to login with the username and a valid token for that username?

GQH1993 · 2020-04-03T00:43:46Z

That's right. I agree with you completely.

GQH1993 · 2020-04-03T00:46:20Z

I think better compatibility will extend the life of the software，I recommend that you support additional ways to clone code,This is also the purpose of my modification

techknowlogick

You could pass token as http://[email protected]/api/v1/version with no password currently, and as such I am hesitant to include this change.

GQH1993 · 2020-04-03T04:11:33Z

Under the current logic, you can only support clone code with a token at the username location；And I think it is necessary to recognize the token in the password position, which will improve our compatibility and user experience；Just like the scene, we now use is http://oauth2:[email protected]/api/v1/version to clone code

GQH1993 · 2020-04-03T04:12:37Z

I recommend that you consider supporting tokens in the password position

zeripath · 2020-04-03T14:45:01Z

We do - but with the correct username. That's what @techknowlogick was saying above.

Would it be possible to explain why you have the fake username oauth2? Is this fake username provided from some other thing? I guess we could add another option but it we're loathe to end-up with an explosion of options.

Do Github/Bitbucket/Gitlab do this?

zeripath · 2020-04-03T14:45:59Z

I honestly don't want to be awkward just want to do the correct thing.

GQH1993 · 2020-04-04T00:56:43Z

I agree with you,this form like ’http://oauth2:[email protected]/api/v1/version‘ is used in gitlab clone code;

GQH1993 · 2020-04-04T06:26:20Z

We do - but with the correct username. That's what @techknowlogick was saying above.

Would it be possible to explain why you have the fake username oauth2? Is this fake username provided from some other thing? I guess we could add another option but it we're loathe to end-up with an explosion of options.

Do Github/Bitbucket/Gitlab do this?

This form like ’http://oauth2:[email protected]/api/v1/version‘ is used in gitlab clone code;

zeripath · 2020-04-04T15:16:39Z

OK, So looking again at our AccessToken structure - I don't think we can safely do this with our AccessTokens.

The tokens are structureless and are essentially just a password - so there is no guarantee that an accesstoken is unique. They're not OAuth2 tokens.

I think you need to look at how you're generating these tokens - as they're not OAuth2 tokens. You wouldn't have to change anything if you were using the OAuth2 grant system.

zeripath · 2020-04-08T16:29:27Z

So I've actually looked a bit further into this. I was mistaken slightly earlier.

It would be perfectly consistent to simply remove the username check completely from the accesstoken checks. The situation we have at present is inconsistent.

What I'm going to do is to put up two PRs:

Add two options REQUIRE_PROVIDED_USERNAME_MATCHES_TOKEN & REQUIRE_USERNAME_WITH_TOKEN and the resultant changes these would require. Add settings: RequireProvidedUsernameMatchesToken and RequireUsernameWithToken #11014
Remove the checks on username. Remove check on username when using AccessToken authentication for the API #11015

Sorry to have misunderstood your requests earlier.

GQH1993 · 2020-04-09T01:34:38Z

So I've actually looked a bit further into this. I was mistaken slightly earlier.

It would be perfectly consistent to simply remove the username check completely from the accesstoken checks. The situation we have at present is inconsistent.

What I'm going to do is to put up two PRs:

Add two options REQUIRE_PROVIDED_USERNAME_MATCHES_TOKEN & REQUIRE_USERNAME_WITH_TOKEN and the resultant changes these would require. Add settings: RequireProvidedUsernameMatchesToken and RequireUsernameWithToken #11014

Remove the checks on username. Remove check on username when using AccessToken authentication for the API #11015

Sorry to have misunderstood your requests earlier.

It doesn't matter. Do you mean that I need to make more modifications according to the above two pr now?

we need use format as 'git clone https://oauth2:<token>@gitea.com/us…

36bd391

…ername/testcode.git' to clone code ,and failure occurred in the process!

GQH1993 mentioned this pull request Mar 31, 2020

can not use "https://oauth2:<token>@url" to clone code #10903

Closed

7 tasks

GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Mar 31, 2020

zeripath suggested changes Mar 31, 2020

View reviewed changes

we need use format as 'git clone https://oauth2:<token>@gitea.com/us…

9e80b56

…ername/testcode.git' to clone code ,and failure occurred in the process!

we need use format as 'git clone https://oauth2:<token>@gitea.com/us…

b7a02ad

…ername/testcode.git' to clone code ,and failure occurred in the process!

GQH1993 requested a review from zeripath April 2, 2020 03:05

techknowlogick requested changes Apr 3, 2020

View reviewed changes

This was referenced Apr 8, 2020

Add settings: RequireProvidedUsernameMatchesToken and RequireUsernameWithToken #11014

Closed

Remove check on username when using AccessToken authentication for the API #11015

Merged

jolheiser closed this in #11015 Apr 14, 2020

go-gitea locked and limited conversation to collaborators Nov 24, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

we need use format as 'git clone https://oauth2:<token>@gitea.com/u… #10902

we need use format as 'git clone https://oauth2:<token>@gitea.com/u… #10902

GQH1993 commented Mar 31, 2020

codecov-io commented Mar 31, 2020

zeripath commented Mar 31, 2020

zeripath left a comment

GQH1993 commented Mar 31, 2020 via email

zeripath commented Mar 31, 2020

zeripath commented Mar 31, 2020

GQH1993 commented Mar 31, 2020 via email

GQH1993 commented Mar 31, 2020 via email

GQH1993 commented Mar 31, 2020

GQH1993 commented Mar 31, 2020

zeripath commented Mar 31, 2020

GQH1993 commented Mar 31, 2020

GQH1993 commented Mar 31, 2020

zeripath commented Apr 1, 2020

GQH1993 commented Apr 3, 2020

GQH1993 commented Apr 3, 2020

techknowlogick left a comment

GQH1993 commented Apr 3, 2020

GQH1993 commented Apr 3, 2020

zeripath commented Apr 3, 2020

zeripath commented Apr 3, 2020

GQH1993 commented Apr 4, 2020

GQH1993 commented Apr 4, 2020

zeripath commented Apr 4, 2020

zeripath commented Apr 8, 2020

GQH1993 commented Apr 9, 2020

we need use format as 'git clone https://oauth2:<token>@gitea.com/u… #10902

we need use format as 'git clone https://oauth2:<token>@gitea.com/u… #10902

Conversation

GQH1993 commented Mar 31, 2020

codecov-io commented Mar 31, 2020

Codecov Report

zeripath commented Mar 31, 2020

zeripath left a comment

Choose a reason for hiding this comment

GQH1993 commented Mar 31, 2020 via email

zeripath commented Mar 31, 2020

zeripath commented Mar 31, 2020

GQH1993 commented Mar 31, 2020 via email

GQH1993 commented Mar 31, 2020 via email

GQH1993 commented Mar 31, 2020

GQH1993 commented Mar 31, 2020

zeripath commented Mar 31, 2020

GQH1993 commented Mar 31, 2020

GQH1993 commented Mar 31, 2020

zeripath commented Apr 1, 2020

GQH1993 commented Apr 3, 2020

GQH1993 commented Apr 3, 2020

techknowlogick left a comment

Choose a reason for hiding this comment

GQH1993 commented Apr 3, 2020

GQH1993 commented Apr 3, 2020

zeripath commented Apr 3, 2020

zeripath commented Apr 3, 2020

GQH1993 commented Apr 4, 2020

GQH1993 commented Apr 4, 2020

zeripath commented Apr 4, 2020

zeripath commented Apr 8, 2020

GQH1993 commented Apr 9, 2020