fix: require state parameter in OAuth GET callback

[mason5052]

Description of the Change

Problem

The AuthLoginGetCallback handler in backend/pkg/server/services/auth.go accepts requests with an empty state query parameter, bypassing CSRF validation. The original condition:

if queryState := c.Query("state"); queryState != "" && queryState != state.Value {

When state is absent or empty, queryState != "" evaluates to false, short-circuiting the entire condition. The handler proceeds to exchange the authorization code without verifying the OAuth state.

The AuthLoginPostCallback handler correctly validates with if data.State != state.Value { which rejects empty values.

Solution

Split the compound condition into two explicit checks:

1. Reject missing state parameter
2. Reject mismatched state value

This aligns the GET callback with the POST callback's strict validation.

Ref: #101

Type of Change

• Bug fix (non-breaking change which fixes an issue)
• Security update

Areas Affected

• Core Services (Frontend UI/Backend API)

Testing and Verification

Test Configuration

PentAGI Version: v1.1.0 (master @ f111863)
Docker Version: N/A (code review)
Host OS: Windows 11
LLM Provider: N/A

Test Steps

1. Review AuthLoginGetCallback (auth.go:331) -- compound condition allows empty state bypass
2. Review AuthLoginPostCallback (auth.go:386) -- strict data.State != state.Value check
3. Apply fix: split into empty state rejection + mismatch rejection
4. Verify fix matches POST handler pattern

Test Results

• Before: GET callback with no state parameter proceeds to token exchange
• After: Missing state returns ErrAuthInvalidAuthorizationState
• Mismatched state still returns error; valid state proceeds normally

Security Considerations

CSRF vulnerability fix. Without state validation, an attacker could craft a callback URL with a valid authorization code but no state parameter, associating the attacker's OAuth identity with the victim's session.

No new dependencies. No permission changes.

Performance Impact

Negligible -- one additional string comparison on the OAuth callback path.

Deployment Notes

Backward-compatible. Legitimate OAuth flows already include the state parameter.

Checklist

Code Quality

• My code follows the project's coding standards
• I have added/updated necessary documentation
• I have added tests to cover my changes
• All new and existing tests pass
• I have run go fmt and go vet (for Go code)

Security

• I have considered security implications
• Changes maintain or improve the security model
• Sensitive information has been properly handled

Compatibility

• Changes are backward compatible
• Dependencies are properly updated

Documentation

• Documentation is clear and complete
• Comments are added for non-obvious code

Additional Notes

Addresses the CSRF state validation bypass from the security audit (issue #101, item 5).

[Copilot]

Pull request overview

This pull request fixes a critical CSRF vulnerability in the OAuth GET callback handler where the state parameter validation could be bypassed by omitting or providing an empty state value. The fix splits the compound boolean condition into two explicit checks that first reject empty state parameters and then validate the state value matches the stored cookie, bringing it into alignment with the POST callback handler's validation logic.

Changes:

• Split OAuth state validation in AuthLoginGetCallback from a single compound condition to two explicit checks
• First check rejects missing/empty state parameter with appropriate error logging and response
• Second check validates state parameter matches stored cookie value

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[asdek]

thanks for the PR!

I'll check it on side, for now let it be in a temporary branch, I gonna to merge it with other changes after testing.