docker login reports sucessful login with incorrect password / token

[noerw]

Description

$ docker login localhost:3000
Username: foobar
Password: 
Login Succeeded

↑ this should fail as i entered gibberish for the password

$ docker push localhost:3000/foobar/qgis
Using default tag: latest
The push refers to repository [localhost:3000/foobar/qgis]
5a1bd4bee150: Layer already exists 
1b04ddff91f2: Layer already exists 
4dd0c5812fd4: Layer already exists 
unauthorized: authentication required

↑ ACLs seem to generally work at least ;)

Gitea Version

692707f

Can you reproduce the bug on the Gitea demo site?

Yes

Log Gist

No response

Screenshots

No response

Git Version

No response

Operating System

No response

How are you running Gitea?

build from source

Database

SQLite

[algernon]

FWIWI, this appears to be the relevant code:

gitea/routers/api/packages/container/container.go

Lines 137 to 152 in ff9b6fa

	func Authenticate(ctx *context.Context) {
	u := ctx.Doer
	if u == nil {
	u = user_model.NewGhostUser()
	}
	token, err := packages_service.CreateAuthorizationToken(u)
	if err != nil {
	apiError(ctx, http.StatusInternalServerError, err)
	return
	}
	ctx.JSON(http.StatusOK, map[string]string{
	"token": token,
	})
	}

I would assume Doer is nil here, because of:

gitea/routers/api/packages/api.go

Lines 254 to 256 in ff9b6fa

	r.Use(func(ctx *context.Context) {
	ctx.Doer = authGroup.Verify(ctx.Req, ctx.Resp, ctx, ctx.Session)
	})

Thus, since Doer is nil, it the first block of code uses the ghost user, and thus, succeeds. If my assumption is correct, then it should treat Doer being nil as an error and 403.

Mind you, this is just my quick impression of the code, I have not verified either of the above assumptions.

[algernon]

FWIW, I have a fix prepared for this, PR will be coming up shortly.

[algernon]

...I spoke too soon. The ghost user is required for anonymous pulls, which also hit the token URL, so just going off of ctx.Doer being nil alone is incorrect. I'll have a PR up shortly anyway. :)

[lunny]

The Auth interface need to be refactored to have a new return argument which indicate should we continue the next auth method.