Wrong HTTP Status 403

[Flomix]

When accessing a secured API endpoint with no or an invalid access token a 403 is returned.

That is wrong, a 403 should only be returned when the token is valid, but the permissions are missing. 401 means not Authenticated, try again with correct authentication. 403 means authenticated but not authorized. Retry will NOT help.

As per RFC 7235:

401 Unauthorized The 401 (Unauthorized) status code indicates that the request has not been applied because it lacks valid authentication credentials for the target resource. The server generating a 401 response MUST send a WWW-Authenticate header field (Section 4.1) containing at least one challenge applicable to the target resource.

If the request included authentication credentials, then the 401 response indicates that authorization has been refused for those credentials. The user agent MAY repeat the request with a new or replaced Authorization header field (Section 4.2).

And in the same RFC:

A server that receives valid credentials that are not adequate to gain access ought to respond with the 403 (Forbidden) status code (Section 6.5.3 of [RFC7231]).

And in RFC7231

403 Forbidden

[...] If authentication credentials were provided in the request, the server considers them insufficient to grant access. The client SHOULD NOT automatically repeat the request with the same credentials. The client MAY repeat the request with new or different credentials. However, a request might be forbidden for reasons unrelated to the credentials.

Expected: HTTP 401 Unauthorized WWW-Authenticate: Token

[darthmaim]

Relevant gitter discussion:

Flomix @Flomix May 24 16:56 just out of curiosity... when I request i.e. https://api.guildwars2.com/v2/account without a token I get a 403. Isn't that the wrong code, shouldn't I get a 401?

darthmaim @darthmaim May 24 16:57 you are probably right

Daniel Snider @dsnider-anet May 24 18:41 hmm, did I do that? That looks like the new error I added after a subtoken check fails... oh, crud, I think I see what I did:

if (!isRegularApiKey(accessToken)) {
  // return 403 if the subtoken decoding/validating is unsuccessful
}

!isRegularApiKey(null) == true

[ChalyFlavour]

While refreshing the API data in some sort of local cache most if us have to handle a variety of unexpected answers, most of them temporary and some key-specific, sometimes after GW updates or addons, but even random. Some of those results are blank like ‘{}’ or ‘error: “invalid data”’ (while using new or older API keys), we’ve even seen simple blanks in the last years. You’re completely right that the 403 code should be a 401, but one should be prepared for even more unexpected answers.

[Flomix]

If I did unterstand Daniel correctly that behavior changed only lately. But I don't know what the actual return was before the subtoken addition, I just stumbled across it because I got a 403 when I didn't expect that.

[dsnider-anet]

I've pushed a change that I think should fix this. Could you verify please?

[sliekens]

@dsnider-anet confirmed, I'm getting 401s now. Thanks.

[Flomix]

Test

• [x] Open endpoint, no token: requested resource (200) - OK
• [x] Open endpoint, wrong token: requested resource (200) - OK
• [x] Open endpoint, existing token: requested resource (200) - OK
• [x] Restricted endpoint, no token: 401 "Invalid access token", WWW-Authenticate: Bearer - OK
• [x] Restricted endpoint, invalid token: 401 "Invalid access token", WWW-Authenticate: Bearer - OK
• [ ] Restricted endpoint, wrong token (double-UUID-structure, but with changed value) 400 "Invalid key" - I would expect a 401 as well. I assume the token contains a checksum, like first UUID is a random value, the second is something like an HMAC, and two random UUIDs fail differently than a different string or an invalidated token?
• [ ] Restricted endpoint, invalidated token (created an actual token and deleted it again): 400 "Invalid key" Same as above, so the assumption with the HMAC seems wrong. I would also expect a 401 here, the same behavior as with "abc" as token. If you want to give the user details about the token structure like you do now I suggest using the message text like you do now, but stick with 401. But I would simply treat all non-existing tokens the same, no matter if they match the double-UUID-structure or not.
• [x] Restricted endpoint, valid token: requested resource (200) - OK
• [x] Restricted endpoint, valid token with missing permission: 403 "requires scope progression" - OK

[Flomix]

I didn't try the behavior with the new JSON Web Tokens, since I did not use them before. But I guess I would expect the same behavior; for a valid JWT (correct hash, not expired) with missing permissions a 403, all other cases (invalid JWT, syntactical correct but expired, wrong hash etc.) a 401.