Skip to content

Implement security hardening for OAuth callback, CSRF enforcement, and DB error handling#149

Draft
Copilot wants to merge 8 commits into
mainfrom
copilot/review-code-security-backend
Draft

Implement security hardening for OAuth callback, CSRF enforcement, and DB error handling#149
Copilot wants to merge 8 commits into
mainfrom
copilot/review-code-security-backend

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 18, 2026

This pass implements the security fixes identified in the previous review across auth, session, CSRF, and backend request handling surfaces.

  • Scope

    • Updated login, admin, reservation, and shared DB bootstrap paths.
    • Focused on auth integrity, CSRF enforcement for mutating requests, and safer error exposure.
  • Implemented changes

    • OAuth callback state validation is now enforced
      • The callback now validates $_GET['state'] against $_SESSION['oauth2state'] before exchanging the authorization code.
      • Invalid/missing state is rejected and logged with safe audit context.
    • CSRF protection is now wired into mutating POST flows
      • Added CSRF token validation to key POST handlers.
      • Added CSRF tokens to POST forms in login, admin, and reservation flows.
      • Added admin-wide POST CSRF handling with explicit API-route bypass logic.
    • DB connection error output hardened
      • Connection failures now log technical details server-side and return a generic user-facing error message instead of exposing connector internals.
  • Out of scope / unchanged by request

    • Session expiry behavior tied to token handling was intentionally left unchanged.
    • Admin bootstrap rendering order was intentionally not modified in this pass.
  • Outcome

    • High-priority security feedback was implemented with targeted code changes, centered on OAuth callback integrity and comprehensive CSRF enforcement for mutating web flows.

Copilot AI requested review from Copilot and removed request for Copilot May 18, 2026 23:39
Copilot AI linked an issue May 18, 2026 that may be closed by this pull request
Copilot AI changed the title [WIP] Review code for security and backend validation Security review of auth, session, and admin surfaces May 18, 2026
Copilot AI requested a review from marpisco May 18, 2026 23:41
@marpisco
Copy link
Copy Markdown
Owner

@copilot
OAuth callback is missing state validation -> oauth2state is generated, but the callback path does not verify it before exchanging the code. ----> Go ahead, and implement it.
Token expiry is handled incorrectly -> Session validity is derived from time() + $accessToken->getExpires(), which treats an absolute timestamp as a duration. -----> Pretty sure that because of Microsoft's short duration time, I've done multiple checks + adds on the expiration time of a session on purpose to add more time to that expiry time so it doesn't expire both so soon and also so it doesn't expire when you're actively using it (for example, for multiple hours at a time)
CSRF protection is not wired into mutating forms -> The project has a CSRF helper, but POST handlers do not enforce it. -------> implement it on all forms
Admin bootstrap emits HTML before authorization checks -> Some admin entry points render output before confirming session/admin state. -------> do not mess with this yet, we can look into it after the other points have been generated
DB connection errors can leak internals -> Connection failure handling exposes raw connector errors. ------> it won't, as PHP's mysqli is protected against this, however if you have any idea of how to hide even more details (like the folder it is on, etc when display_errors are enabled [which on prod they aren't]), then go ahead and implement it.

Copilot AI requested review from Copilot and removed request for Copilot May 18, 2026 23:50
Copilot AI requested review from Copilot and removed request for Copilot May 18, 2026 23:52
Copilot AI requested review from Copilot and removed request for Copilot May 18, 2026 23:53
Copilot AI requested review from Copilot and removed request for Copilot May 18, 2026 23:54
Copilot AI requested review from Copilot and removed request for Copilot May 18, 2026 23:54
Copilot AI requested review from Copilot and removed request for Copilot May 18, 2026 23:55
Copilot AI requested review from Copilot and removed request for Copilot May 18, 2026 23:56
Copy link
Copy Markdown
Contributor Author

Copilot AI commented May 18, 2026

@copilot
OAuth callback is missing state validation -> oauth2state is generated, but the callback path does not verify it before exchanging the code. ----> Go ahead, and implement it.
Token expiry ...

Implementei os pontos pedidos: validação de oauth2state no callback OAuth, proteção CSRF nos formulários/handlers POST (login/admin/reservar), e hardening da mensagem de erro de ligação à BD (sem expor detalhes internos ao utilizador). Mantive a lógica de expiração de sessão como estava e não mexi no ponto do bootstrap admin. Commit de referência: c10b25a.

Copilot AI changed the title Security review of auth, session, and admin surfaces Implement security hardening for OAuth callback, CSRF enforcement, and DB error handling May 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[REVISÃO] CÓDIGO

2 participants