Skip to content

Remove optional auth mode from achievement code endpoints#253

Open
oPeras1 wants to merge 1 commit into
sinfo:masterfrom
oPeras1:fix-mode-try
Open

Remove optional auth mode from achievement code endpoints#253
oPeras1 wants to merge 1 commit into
sinfo:masterfrom
oPeras1:fix-mode-try

Conversation

@oPeras1
Copy link
Copy Markdown

@oPeras1 oPeras1 commented Apr 20, 2026

This PR fixes a security issue caused by using mode: try in achievement routes protected by team/admin scope.

With mode: authentication failures can still pass through as unauthenticated requests, which makes that anyone can do the request and have the codes of each achievement.

🧙

Copilot AI review requested due to automatic review settings April 20, 2026 13:25
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens the achievement “code” endpoints by removing Hapi auth.mode: 'try', ensuring authentication failures don’t fall through as unauthenticated requests (which could expose achievement codes).

Changes:

  • Removed auth.mode: 'try' from GET /achievements/code and GET /achievements/{id}/code route options.
  • Restores default “required” auth behavior for these team/admin-scoped endpoints.

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

Comment thread server/routes/achievement/handlers.js Outdated
@@ -389,7 +389,6 @@ exports.listWithCode = {
auth: {
strategies: ['default'],
scope: ['team', 'admin'],
Copy link

Copilot AI Apr 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After removing mode: 'try', scope is now the last property in this object but still has a dangling trailing comma. The repo uses standard linting (package.json lint script), which will fail on trailing commas—please remove the trailing comma here to satisfy the style/lint rules.

Suggested change
scope: ['team', 'admin'],
scope: ['team', 'admin']

Copilot uses AI. Check for mistakes.
Comment thread server/routes/achievement/handlers.js
Comment thread server/routes/achievement/handlers.js Outdated
@@ -389,7 +389,6 @@ exports.listWithCode = {
auth: {
strategies: ['default'],
scope: ['team', 'admin'],
Copy link

Copilot AI Apr 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change fixes an auth-mode bypass for the code endpoints; there are existing tests for admin/team/user access, but no regression test asserting that unauthenticated requests (no auth header) are rejected (expected 401) for /achievements/code and /achievements/{id}/code. Adding that test would prevent this vulnerability from reappearing.

Suggested change
scope: ['team', 'admin'],
scope: ['team', 'admin'],
mode: 'required'

Copilot uses AI. Check for mistakes.
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.

2 participants