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

CORS Preflight Fails for API Calls #16100

Closed
2 of 6 tasks
drewmnoel opened this issue Jun 7, 2021 · 6 comments · Fixed by #16524
Closed
2 of 6 tasks

CORS Preflight Fails for API Calls #16100

drewmnoel opened this issue Jun 7, 2021 · 6 comments · Fixed by #16524
Labels

Comments

@drewmnoel
Copy link
Contributor

  • Gitea version (or commit ref): 3607f79
  • Git version: 2.32.0
  • Operating system:
    • Arch Linux
    • go1.16.5 linux/amd64
    • TAGS="bindata sqlite sqlite_unlock_notify" make build
  • Database (use [x]):
    • PostgreSQL
    • MySQL
    • MSSQL
    • SQLite (but shouldn't matter)
  • Can you reproduce the bug at https://try.gitea.io:
    • Yes
    • No (requires CORS enabled in app.ini)
  • Log gist:
    • None, behavior is not logged by gitea

Description

When accessing the API with JavaScript and an OAuth token, a CORS preflight is triggered due to the precense of the Authorization: header. The CORS middleware denies the preflight, returning only the Vary: header, rather than the expected set of Access-Control-Allow-. An example request (note: organization and repo do not need to exist):

$ curl -v 'http://localhost:3000/api/v1/repos/anyorg/anyrepo/contents/anything' -X OPTIONS -H 'Access-Control-Request-Method: OPTIONS' -H 'Access-Control-Request-Headers: authorization' -H 'Origin: localhost:3000'
*   Trying ::1:3000...
* Connected to localhost (::1) port 3000 (#0)
> OPTIONS /api/v1/repos/anyorg/anyrepo/contents/anything HTTP/1.1
> Host: localhost:3000
> User-Agent: curl/7.77.0
> Accept: */*
> Access-Control-Request-Method: OPTIONS
> Access-Control-Request-Headers: authorization
> Origin: localhost:3000
> 
* Mark bundle as not supporting multiuse
< HTTP/1.1 200 OK
< Set-Cookie: i_like_gitea=[...]; Path=/; HttpOnly; SameSite=Lax
< Vary: Origin
< Vary: Access-Control-Request-Method
< Vary: Access-Control-Request-Headers
< X-Content-Type-Options: nosniff
< Date: Mon, 07 Jun 2021 18:57:42 GMT
< Content-Length: 0
< 
* Connection #0 to host localhost left intact

It appears that gitea may not be informing the CORS middleware of the "Authorization" header for API calls:

gitea/routers/api/v1/api.go

Lines 564 to 573 in cbf3083

if setting.CORSConfig.Enabled {
m.Use(cors.Handler(cors.Options{
//Scheme: setting.CORSConfig.Scheme, // FIXME: the cors middleware needs scheme option
AllowedOrigins: setting.CORSConfig.AllowDomain,
//setting.CORSConfig.AllowSubdomain // FIXME: the cors middleware needs allowSubdomain option
AllowedMethods: setting.CORSConfig.Methods,
AllowCredentials: setting.CORSConfig.AllowCredentials,
MaxAge: int(setting.CORSConfig.MaxAge.Seconds()),
}))
}

If this call is updated to include Debug: true, the middleware will print to standard output when it runs, which shows that the authorization header is not permitted:

2021/06/07 14:57:40 cmd/web.go:81:runWeb() [I] Starting Gitea on PID: 561602
2021/06/07 14:57:40 ...dules/setting/git.go:101:newGit() [I] Git Version: 2.32.0, Wire Protocol Version 2 Enabled
2021/06/07 14:57:40 cmd/web.go:125:runWeb() [I] Global init
2021/06/07 14:57:40 ...dules/setting/git.go:101:newGit() [I] Git Version: 2.32.0, Wire Protocol Version 2 Enabled
2021/06/07 14:57:40 routers/init.go:134:GlobalInit() [T] AppPath: [...]/gitea
2021/06/07 14:57:40 routers/init.go:135:GlobalInit() [T] AppWorkPath: [...]/
2021/06/07 14:57:40 routers/init.go:136:GlobalInit() [T] Custom path: [...]/custom
2021/06/07 14:57:40 routers/init.go:137:GlobalInit() [T] Log path: [...]/data/log
2021/06/07 14:57:40 routers/init.go:49:checkRunMode() [I] Run Mode: Prod
[cors] 2021/06/07 14:58:53 Handler: Preflight request
[cors] 2021/06/07 14:58:53 Preflight aborted: headers '[Authorization]' not allowed
2021/06/07 14:58:53 Started OPTIONS /api/v1/repos/anyorg/anyrepo/contents/anything for [::1]:49478
2021/06/07 14:58:53 Completed OPTIONS /api/v1/repos/anyorg/anyrepo/contents/anything 200 OK in 131.77µs
@drewmnoel
Copy link
Contributor Author

This issue occurs for me in every commit since the middleware switchover (6433ba0). For what it's worth, the old CORS middleware would just reflect the Access-Control-Allow-Header field, while the new one requires a whitelist to be given.

@zeripath
Copy link
Contributor

zeripath commented Jun 7, 2021

Looking at:

gitea/routers/api/v1/api.go

Lines 564 to 573 in cbf3083

if setting.CORSConfig.Enabled {
m.Use(cors.Handler(cors.Options{
//Scheme: setting.CORSConfig.Scheme, // FIXME: the cors middleware needs scheme option
AllowedOrigins: setting.CORSConfig.AllowDomain,
//setting.CORSConfig.AllowSubdomain // FIXME: the cors middleware needs allowSubdomain option
AllowedMethods: setting.CORSConfig.Methods,
AllowCredentials: setting.CORSConfig.AllowCredentials,
MaxAge: int(setting.CORSConfig.MaxAge.Seconds()),
}))
}

It appears that we're not setting the AllowHeaders or ExposedHeaders options.

Whilst we could make these user definable - I suspect that that is incorrect - so we need to go through the API and work out which headers needs to be settable by and exposed to the user.

@zeripath
Copy link
Contributor

zeripath commented Jun 8, 2021

OK so most of the time we explicitly set the correct Access-Control-Expose-Headers - but the Access-Control-Allow-Headers thing needs to be set.

@zeripath
Copy link
Contributor

@drewmnoel having just fixed #16350 where I noted that the issue was the lack of routes using the OPTIONS method and looking again at your opening comment I see that you're also explicitly mentioning OPTIONS.

We don't currently support OPTIONS on our API routes which would explain why these are failing and our Swagger API doesn't even suggest that we do support OPTIONS.

So - I think we need some explanation as to why and what you're using OPTIONS for.

@zeripath
Copy link
Contributor

@zeripath
Copy link
Contributor

This is going to require a lot of work

zeripath added a commit to zeripath/gitea that referenced this issue Jul 23, 2021
Set AllowedHeaders on API CORS handler and add missing Access-Control-Expose-Headers
to pull API.

Fix go-gitea#16100

Signed-off-by: Andrew Thornton <[email protected]>
zeripath added a commit to zeripath/gitea that referenced this issue Aug 4, 2021
Backport go-gitea#16524

Set AllowedHeaders on API CORS handler and add missing Access-Control-Expose-Headers
to pull API.

Fix go-gitea#16100

Signed-off-by: Andrew Thornton <[email protected]>
6543 pushed a commit that referenced this issue Aug 4, 2021
Set AllowedHeaders on API CORS handler and add missing Access-Control-Expose-Headers
to pull API.

Fix #16100

Signed-off-by: Andrew Thornton <[email protected]>
6543 pushed a commit that referenced this issue Aug 4, 2021
Backport #16524

Set AllowedHeaders on API CORS handler and add missing Access-Control-Expose-Headers
to pull API.

Fix #16100

Signed-off-by: Andrew Thornton <[email protected]>
@go-gitea go-gitea locked and limited conversation to collaborators Oct 19, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants