jamesfredley commented on PR #15812:
URL: https://github.com/apache/grails-core/pull/15812#issuecomment-4997826034
Worth clarifying the current state, because "we removed the ability to
deploy to a github repo" is only half-true in this repo - and the half that's
left is the risky one.
What was removed is the **UI**: the "Create in GitHub" action lives in the
separate `grails-forge-ui` (React) repo, which is where that removal happened.
But the **backend** GitHub create/deploy flow is still fully present and wired
here in `grails-core`:
- `grails-forge-api`: `.../create/github/GitHubCreateController` (routes
`/github/{type}/{name}` taking `code`/`state`), `GitHubRedirectService` (builds
the OAuth authorize URL with `client_id`/`redirect_uri`/`scope`/`state`),
`GitHubCreateService` (token exchange -> repo creation -> push),
`GitHubCreateOperation`.
- `grails-forge-core`: `.../client/github/oauth/GitHubOAuthClient` +
`GitHubOAuthOperations`, `GitHubUtil`.
- `grails-forge-web-netty/src/main/resources/application.yml`: live
`github-oauth` / `github-api-v3` service URLs and the OAuth
client-id/secret/redirect config.
And critically, the `state` is generated (`UUID.randomUUID()`) but **not
validated server-side** - which is exactly the CSRF gap this PR closes.
So we currently ship live, reachable OAuth endpoints with an unhardened
`state`. That leaves two coherent options - and the current in-between (UI
gone, backend live + unhardened) is the worst of the three:
1. **Retire it fully** - remove the backend too: the `grails-forge-api`
`.../create/github` package, the `grails-forge-core` GitHub OAuth client, and
the `application.yml` GitHub config, matching the UI removal. That's a
separate, larger removal PR.
2. **Keep and harden** - if those endpoints stay shipped, they're live CSRF
surface and the state validation here is the correct fix.
I'm happy either way. If the decision is to retire, I'll convert this into
the backend-removal PR instead. But if any of those `/github/...` endpoints
remain in a released Forge, I'd merge the state validation first so we're not
shipping an unhardened OAuth callback. Which direction do you want?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
