diff --git a/.devcontainer/docker-compose.mysql.yml b/.devcontainer/docker-compose.mysql.yml index bbeffdf61..faced370c 100644 --- a/.devcontainer/docker-compose.mysql.yml +++ b/.devcontainer/docker-compose.mysql.yml @@ -14,6 +14,10 @@ services: HASHTOPOLIS_DB_HOST: hashtopolis-db-dev HASHTOPOLIS_DB_DATABASE: hashtopolis HASHTOPOLIS_APIV2_ENABLE: 1 + # The refresh token cookie is only sent cross-origin when the API names a concrete allowed + # origin instead of a wildcard, so the dev frontend on :4200 has to be named here. + HASHTOPOLIS_BACKEND_URL: http://localhost:8080 + HASHTOPOLIS_FRONTEND_URLS: http://localhost:4200,http://127.0.0.1:4200 depends_on: - hashtopolis-db-dev ports: diff --git a/.devcontainer/docker-compose.postgres.yml b/.devcontainer/docker-compose.postgres.yml index a0b2e13e9..ed5bc5400 100644 --- a/.devcontainer/docker-compose.postgres.yml +++ b/.devcontainer/docker-compose.postgres.yml @@ -14,6 +14,10 @@ services: HASHTOPOLIS_DB_HOST: hashtopolis-db-dev HASHTOPOLIS_DB_DATABASE: hashtopolis HASHTOPOLIS_APIV2_ENABLE: 1 + # The refresh token cookie is only sent cross-origin when the API names a concrete allowed + # origin instead of a wildcard, so the dev frontend on :4200 has to be named here. + HASHTOPOLIS_BACKEND_URL: http://localhost:8080 + HASHTOPOLIS_FRONTEND_URLS: http://localhost:4200,http://127.0.0.1:4200 depends_on: - hashtopolis-db-dev ports: diff --git a/.github/openapi/spectral-hashtopolis.yml b/.github/openapi/spectral-hashtopolis.yml index b7cb9f4ed..839b57616 100644 --- a/.github/openapi/spectral-hashtopolis.yml +++ b/.github/openapi/spectral-hashtopolis.yml @@ -265,3 +265,13 @@ overrides: content-type: off 400-response-code: off 403-response-code: off + + # /api/v2/auth/refresh trades the refresh token cookie for a new access token and, on DELETE, ends + # the session behind it. Like the token endpoint it is not a JSON:API resource route and answers a + # plain application/json body (see token.routes.php). It does document 403, so unlike the token + # endpoint the 403 rule still holds and stays on. + - files: + - "**#/paths/~1api~1v2~1auth~1refresh" + rules: + content-type: off + 400-response-code: off diff --git a/ci/apiv2/test_refresh_token.py b/ci/apiv2/test_refresh_token.py new file mode 100644 index 000000000..6647467c4 --- /dev/null +++ b/ci/apiv2/test_refresh_token.py @@ -0,0 +1,195 @@ +import base64 +import json + +import requests + +from utils import BaseTest, get_test_config, get_hashtopolis_uri + +AUTH_URI = get_hashtopolis_uri() + '/api/v2/auth/token' +REFRESH_URI = get_hashtopolis_uri() + '/api/v2/auth/refresh' + +COOKIE_NAME = 'refreshToken' +COOKIE_PATH = '/api/v2/auth/refresh' + + +def _credentials(): + cfg = get_test_config() + return cfg['username'], cfg['password'] + + +def _login(session=None): + """Log in and return the response; the refresh token lands in the session's cookie jar.""" + session = session or requests.Session() + response = session.post(AUTH_URI, auth=_credentials()) + return session, response + + +def _refresh_token_of(session): + return session.cookies.get(COOKIE_NAME, path=COOKIE_PATH) + + +def _jwt_payload(token): + """Decode a JWT payload without verifying it; the server already vouched for the signature.""" + payload_b64 = token.split('.')[1] + payload_b64 += '=' * (-len(payload_b64) % 4) + return json.loads(base64.urlsafe_b64decode(payload_b64)) + + +class RefreshTokenTest(BaseTest): + def test_login_sets_refresh_cookie(self): + session, response = _login() + self.assertEqual(response.status_code, 201, msg=response.text) + + cookie = response.headers['Set-Cookie'] + self.assertIn(COOKIE_NAME + '=', cookie) + self.assertIn('HttpOnly', cookie) + self.assertIn('SameSite=', cookie) + self.assertIn('Path=' + COOKIE_PATH, cookie) + self.assertIsNotNone(_refresh_token_of(session)) + + def test_cookie_is_secure_over_https(self): + """Secure follows the scheme, so that a plain HTTP deployment does not lose the cookie.""" + _, response = _login() + cookie = response.headers['Set-Cookie'] + + if get_hashtopolis_uri().startswith('https://'): + self.assertIn('Secure', cookie) + else: + self.assertNotIn('Secure', cookie) + + def test_login_body_never_carries_the_refresh_token(self): + """The token is HttpOnly-cookie-only on purpose, so that XSS cannot read it.""" + session, response = _login() + body = response.json() + + self.assertIn('token', body) + self.assertIn('expires', body) + self.assertNotIn('refreshToken', body) + self.assertNotIn(_refresh_token_of(session), response.text) + + def test_issued_tokens_declare_themselves_as_access_tokens(self): + """Every token is signed with the same key, so only the claim says what it may be spent on.""" + _, login = _login() + self.assertEqual(_jwt_payload(login.json()['token'])['type'], 'access') + + session, _ = _login() + refreshed = session.post(REFRESH_URI) + self.assertEqual(_jwt_payload(refreshed.json()['token'])['type'], 'access') + + def test_refresh_returns_a_usable_access_token(self): + session, login = _login() + + response = session.post(REFRESH_URI) + self.assertEqual(response.status_code, 201, msg=response.text) + + body = response.json() + self.assertIn('token', body) + self.assertIn('expires', body) + self.assertNotEqual(body['token'], login.json()['token']) + + probe = requests.get(get_hashtopolis_uri() + '/api/v2/ui/users', + headers={'Authorization': 'Bearer ' + body['token']}) + self.assertEqual(probe.status_code, 200, msg=probe.text) + + def test_refresh_needs_no_access_token(self): + """The endpoint has to work once the access token is gone, so it must not require one.""" + session, _ = _login() + + response = session.post(REFRESH_URI, headers={'Authorization': 'Bearer not-a-token'}) + self.assertEqual(response.status_code, 201, msg=response.text) + + def test_refresh_rotates_the_cookie(self): + session, _ = _login() + before = _refresh_token_of(session) + + session.post(REFRESH_URI) + after = _refresh_token_of(session) + + self.assertIsNotNone(after) + self.assertNotEqual(before, after) + + def test_refresh_without_cookie_is_unauthorized(self): + response = requests.post(REFRESH_URI) + self.assertEqual(response.status_code, 401, msg=response.text) + + def test_refresh_with_unknown_token_is_unauthorized(self): + response = requests.post(REFRESH_URI, cookies={COOKIE_NAME: 'not-a-token'}) + self.assertEqual(response.status_code, 401, msg=response.text) + + def test_replaying_a_consumed_token_kills_the_session(self): + session, _ = _login() + stolen = _refresh_token_of(session) + + self.assertEqual(session.post(REFRESH_URI).status_code, 201) + successor = _refresh_token_of(session) + + # A second exchange of the same token means two parties hold it, however soon it arrives + replay = requests.post(REFRESH_URI, cookies={COOKIE_NAME: stolen}) + self.assertEqual(replay.status_code, 401, msg=replay.text) + + # ... which takes the untouched successor down with it + after = requests.post(REFRESH_URI, cookies={COOKIE_NAME: successor}) + self.assertEqual(after.status_code, 401, msg=after.text) + + def test_sessions_are_independent(self): + first, _ = _login() + second, _ = _login() + + self.assertEqual(first.post(REFRESH_URI).status_code, 201) + self.assertEqual(second.post(REFRESH_URI).status_code, 201) + + def test_logout_ends_only_its_own_session(self): + kept, _ = _login() + ended, _ = _login() + + # Capture while it is still live: the logout response expires the cookie, so reading it from + # the jar afterwards yields None and the rejection below would prove nothing but its absence + revoked = _refresh_token_of(ended) + self.assertIsNotNone(revoked) + + response = ended.delete(REFRESH_URI) + self.assertEqual(response.status_code, 204, msg=response.text) + self.assertIn('Max-Age=0', response.headers['Set-Cookie']) + self.assertIsNone(_refresh_token_of(ended), 'logout should also drop the cookie client-side') + + # The token the client actually held is now refused by the server, not merely forgotten + replay = requests.post(REFRESH_URI, cookies={COOKIE_NAME: revoked}) + self.assertEqual(replay.status_code, 401, msg=replay.text) + + self.assertEqual(kept.post(REFRESH_URI).status_code, 201) + + def test_a_cross_site_refresh_cannot_spend_the_token(self): + """A forged cross-site request must be refused before it takes effect. + + The refusal alone is not enough: these handlers authenticate from the cookie, so a page + elsewhere can have the browser make the call. The attacker never reads the reply, but if the + token is spent first, the victim's next renewal looks like a replay and ends every session of + that login. So the test is not that the attacker gets a 403 - it is that the victim's token + still works afterwards. + """ + session, _ = _login() + victims_token = _refresh_token_of(session) + + forged = requests.post(REFRESH_URI, + headers={'Origin': 'https://evil.example'}, + cookies={COOKIE_NAME: victims_token}) + self.assertEqual(forged.status_code, 403, msg=forged.text) + + still_valid = requests.post(REFRESH_URI, cookies={COOKIE_NAME: victims_token}) + self.assertEqual(still_valid.status_code, 201, + msg='the forged request spent the victim token before being refused') + + def test_a_cross_site_logout_cannot_end_the_session(self): + session, _ = _login() + victims_token = _refresh_token_of(session) + + forged = requests.delete(REFRESH_URI, + headers={'Origin': 'https://evil.example'}, + cookies={COOKIE_NAME: victims_token}) + self.assertEqual(forged.status_code, 403, msg=forged.text) + + still_valid = requests.post(REFRESH_URI, cookies={COOKIE_NAME: victims_token}) + self.assertEqual(still_valid.status_code, 201, msg='the forged request revoked the session') + + def test_logout_without_cookie_is_not_an_error(self): + self.assertEqual(requests.delete(REFRESH_URI).status_code, 204) diff --git a/ci/phpunit/dba/CompareAndSetTest.php b/ci/phpunit/dba/CompareAndSetTest.php new file mode 100644 index 000000000..c78a61d0e --- /dev/null +++ b/ci/phpunit/dba/CompareAndSetTest.php @@ -0,0 +1,202 @@ +user = $this->createUser('cas_user'); + $this->agent = $this->createAgent('cas_agent'); + } + + /** + * @throws Exception + */ + public function testTransitionAppliesWhenTheRowStillMatches(): void { + $applied = Factory::getAgentFactory()->compareAndSet( + $this->agent, + [Agent::IS_ACTIVE => $this->agent->getIsActive()], + [Agent::AGENT_NAME => 'renamed'] + ); + + $this->assertTrue($applied); + $this->assertSame('renamed', Factory::getAgentFactory()->get($this->agent->getId())->getAgentName()); + } + + /** + * The point of the primitive: the second caller is told it lost rather than overwriting the first. + * + * @throws Exception + */ + public function testOnlyTheFirstOfTwoCallersWins(): void { + $claim = fn(string $name): bool => Factory::getAgentFactory()->compareAndSet( + $this->agent, + [Agent::USER_ID => null], + [Agent::USER_ID => $this->user->getId(), Agent::AGENT_NAME => $name] + ); + + $this->assertTrue($claim('winner')); + $this->assertFalse($claim('loser')); + $this->assertSame('winner', Factory::getAgentFactory()->get($this->agent->getId())->getAgentName()); + } + + /** + * @throws Exception + */ + public function testTransitionIsRefusedWhenTheRowNoLongerMatches(): void { + $applied = Factory::getAgentFactory()->compareAndSet( + $this->agent, + [Agent::AGENT_NAME => 'some other name'], + [Agent::AGENT_NAME => 'renamed'] + ); + + $this->assertFalse($applied); + $this->assertSame($this->agent->getAgentName(), Factory::getAgentFactory()->get($this->agent->getId())->getAgentName()); + } + + /** + * A null expectation has to become IS NULL: a bound parameter compared with = never matches null, + * so spelling it as a normal comparison would make the transition permanently unreachable. + * + * @throws Exception + */ + public function testNullIsExpectedAsIsNull(): void { + $this->assertNull($this->agent->getUserId()); + + $this->assertTrue(Factory::getAgentFactory()->compareAndSet( + $this->agent, + [Agent::USER_ID => null], + [Agent::USER_ID => $this->user->getId()] + )); + + // ... and once the column is set, the same expectation no longer holds + $this->assertFalse(Factory::getAgentFactory()->compareAndSet( + $this->agent, + [Agent::USER_ID => null], + [Agent::USER_ID => $this->user->getId()] + )); + } + + /** + * Every expectation has to hold, not just one of them. + * + * @throws Exception + */ + public function testAllExpectationsMustHold(): void { + $applied = Factory::getAgentFactory()->compareAndSet( + $this->agent, + [Agent::USER_ID => null, Agent::AGENT_NAME => 'some other name'], + [Agent::AGENT_NAME => 'renamed'] + ); + + $this->assertFalse($applied); + } + + /** + * The row is addressed by its primary key, so a matching expectation on another row changes nothing. + * + * @throws Exception + */ + public function testOnlyTheAddressedRowIsTouched(): void { + $other = $this->createAgent('cas_other'); + + Factory::getAgentFactory()->compareAndSet( + $this->agent, + [Agent::USER_ID => null], + [Agent::AGENT_NAME => 'renamed'] + ); + + $this->assertSame($other->getAgentName(), Factory::getAgentFactory()->get($other->getId())->getAgentName()); + } + + /** + * @throws Exception + */ + public function testNoExpectationsMeansAnUnconditionalUpdate(): void { + $this->assertTrue(Factory::getAgentFactory()->compareAndSet( + $this->agent, + [], + [Agent::AGENT_NAME => 'renamed'] + )); + } + + /** + * @throws Exception + */ + public function testUpdatingNothingIsRejected(): void { + $this->expectException(Exception::class); + + Factory::getAgentFactory()->compareAndSet($this->agent, [Agent::USER_ID => null], []); + } + + /** + * Writing back exactly what was expected changes no row, and MySQL reports a changed-row count + * while PostgreSQL reports a matched one. Rather than answer differently per database, refuse it. + * + * @throws Exception + */ + public function testWritingBackTheExpectedValueIsRejected(): void { + $this->expectException(Exception::class); + + Factory::getAgentFactory()->compareAndSet( + $this->agent, + [Agent::IS_TRUSTED => 1], + [Agent::IS_TRUSTED => 1] + ); + } + + /** + * Only the whole update has to be a no-op to be refused; changing one column while holding another + * to an expected value is the normal shape of a claim. + * + * @throws Exception + */ + public function testHoldingOneColumnWhileChangingAnotherIsAllowed(): void { + $this->assertTrue(Factory::getAgentFactory()->compareAndSet( + $this->agent, + [Agent::IS_TRUSTED => 1, Agent::USER_ID => null], + [Agent::IS_TRUSTED => 1, Agent::USER_ID => $this->user->getId()] + )); + } + + /** + * Booleans are stored as tinyint on MySQL and as a real boolean on PostgreSQL, so a flag flip has + * to survive both dialects. + * + * @throws Exception + */ + public function testBooleanColumnTransitions(): void { + $this->assertEquals(1, $this->agent->getIsTrusted()); + + $this->assertTrue(Factory::getAgentFactory()->compareAndSet( + $this->agent, + [Agent::IS_TRUSTED => 1], + [Agent::IS_TRUSTED => 0] + )); + $this->assertEquals(0, Factory::getAgentFactory()->get($this->agent->getId())->getIsTrusted()); + + // The flag has already been flipped, so the same transition must not apply a second time + $this->assertFalse(Factory::getAgentFactory()->compareAndSet( + $this->agent, + [Agent::IS_TRUSTED => 1], + [Agent::IS_TRUSTED => 0] + )); + } +} diff --git a/ci/phpunit/fixtures/openapi/abortchunk.spec.json b/ci/phpunit/fixtures/openapi/abortchunk.spec.json index 4293eee9e..34f644b0e 100644 --- a/ci/phpunit/fixtures/openapi/abortchunk.spec.json +++ b/ci/phpunit/fixtures/openapi/abortchunk.spec.json @@ -141,6 +141,98 @@ ] } }, + "/api/v2/auth/refresh": { + "post": { + "tags": [ + "Login" + ], + "summary": "Exchange the refresh token cookie for a new access token", + "description": "Reads the refreshToken cookie set by /api/v2/auth/token, rotates it and answers\n with a new access token. Needs no Authorization header, so it keeps working once the previous\n access token has expired. The rotated cookie is returned in Set-Cookie; the token itself is\n never part of the body.", + "responses": { + "201": { + "description": "Success", + "headers": { + "Set-Cookie": { + "description": "The rotated refresh token, scoped to /api/v2/auth/refresh and marked HttpOnly.\n Replaces the cookie sent with the request, which is consumed by this call.", + "schema": { + "type": "string", + "example": "refreshToken=4fa1371293a112224bc930cf9fecd0fd; Path=/api/v2/auth/refresh; Max-Age=1209600; Expires=Wed, 30 Sep 2026 07:02:31 GMT; HttpOnly; SameSite=Strict" + } + } + }, + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Token" + } + } + } + }, + "401": { + "description": "The refresh token is missing, expired, revoked or already used", + "content": { + "application/problem+json": { + "schema": { + "$ref": "#/components/schemas/ErrorResponse" + } + } + } + }, + "403": { + "description": "The user has been deactivated", + "content": { + "application/problem+json": { + "schema": { + "$ref": "#/components/schemas/ErrorResponse" + } + } + } + } + }, + "security": [ + { + "refreshCookie": [] + } + ] + }, + "delete": { + "tags": [ + "Login" + ], + "summary": "Log out", + "description": "Revokes the session the refreshToken cookie belongs to and clears the cookie.\n Sessions on other devices are left alone. Logging out without a cookie is not an error.", + "responses": { + "204": { + "description": "Success", + "headers": { + "Set-Cookie": { + "description": "The refresh token cookie, emptied and expired so the client drops it. Carries\n the same attributes it was set with, which is what makes a browser replace rather than keep it.", + "schema": { + "type": "string", + "example": "refreshToken=; Path=/api/v2/auth/refresh; Max-Age=0; Expires=Wed, 16 Sep 2026 07:02:31 GMT; HttpOnly; SameSite=Strict" + } + } + } + }, + "403": { + "description": "The request origin is not allowed to send credentials", + "content": { + "application/problem+json": { + "schema": { + "$ref": "#/components/schemas/ErrorResponse" + } + } + } + } + }, + "security": [ + { + "refreshCookie": [] + }, + {} + ] + } + }, "/api/v2/helper/importFile": { "post": { "parameters": [ @@ -442,6 +534,12 @@ "type": "http", "description": "Basic Authorization header.", "scheme": "basic" + }, + "refreshCookie": { + "type": "apiKey", + "description": "HttpOnly cookie holding the refresh token, set by /api/v2/auth/token and scoped to /api/v2/auth/refresh.", + "in": "cookie", + "name": "refreshToken" } } } diff --git a/ci/phpunit/fixtures/openapi/config.spec.json b/ci/phpunit/fixtures/openapi/config.spec.json index 99f491137..ff2b93a0f 100644 --- a/ci/phpunit/fixtures/openapi/config.spec.json +++ b/ci/phpunit/fixtures/openapi/config.spec.json @@ -1114,6 +1114,98 @@ ] } }, + "/api/v2/auth/refresh": { + "post": { + "tags": [ + "Login" + ], + "summary": "Exchange the refresh token cookie for a new access token", + "description": "Reads the refreshToken cookie set by /api/v2/auth/token, rotates it and answers\n with a new access token. Needs no Authorization header, so it keeps working once the previous\n access token has expired. The rotated cookie is returned in Set-Cookie; the token itself is\n never part of the body.", + "responses": { + "201": { + "description": "Success", + "headers": { + "Set-Cookie": { + "description": "The rotated refresh token, scoped to /api/v2/auth/refresh and marked HttpOnly.\n Replaces the cookie sent with the request, which is consumed by this call.", + "schema": { + "type": "string", + "example": "refreshToken=4fa1371293a112224bc930cf9fecd0fd; Path=/api/v2/auth/refresh; Max-Age=1209600; Expires=Wed, 30 Sep 2026 07:02:31 GMT; HttpOnly; SameSite=Strict" + } + } + }, + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Token" + } + } + } + }, + "401": { + "description": "The refresh token is missing, expired, revoked or already used", + "content": { + "application/problem+json": { + "schema": { + "$ref": "#/components/schemas/ErrorResponse" + } + } + } + }, + "403": { + "description": "The user has been deactivated", + "content": { + "application/problem+json": { + "schema": { + "$ref": "#/components/schemas/ErrorResponse" + } + } + } + } + }, + "security": [ + { + "refreshCookie": [] + } + ] + }, + "delete": { + "tags": [ + "Login" + ], + "summary": "Log out", + "description": "Revokes the session the refreshToken cookie belongs to and clears the cookie.\n Sessions on other devices are left alone. Logging out without a cookie is not an error.", + "responses": { + "204": { + "description": "Success", + "headers": { + "Set-Cookie": { + "description": "The refresh token cookie, emptied and expired so the client drops it. Carries\n the same attributes it was set with, which is what makes a browser replace rather than keep it.", + "schema": { + "type": "string", + "example": "refreshToken=; Path=/api/v2/auth/refresh; Max-Age=0; Expires=Wed, 16 Sep 2026 07:02:31 GMT; HttpOnly; SameSite=Strict" + } + } + } + }, + "403": { + "description": "The request origin is not allowed to send credentials", + "content": { + "application/problem+json": { + "schema": { + "$ref": "#/components/schemas/ErrorResponse" + } + } + } + } + }, + "security": [ + { + "refreshCookie": [] + }, + {} + ] + } + }, "/api/v2/helper/importFile": { "post": { "parameters": [ @@ -3661,6 +3753,12 @@ "type": "http", "description": "Basic Authorization header.", "scheme": "basic" + }, + "refreshCookie": { + "type": "apiKey", + "description": "HttpOnly cookie holding the refresh token, set by /api/v2/auth/token and scoped to /api/v2/auth/refresh.", + "in": "cookie", + "name": "refreshToken" } } } diff --git a/ci/phpunit/fixtures/openapi/crackerbinarytype.spec.json b/ci/phpunit/fixtures/openapi/crackerbinarytype.spec.json index 4188aebcf..214779658 100644 --- a/ci/phpunit/fixtures/openapi/crackerbinarytype.spec.json +++ b/ci/phpunit/fixtures/openapi/crackerbinarytype.spec.json @@ -1163,6 +1163,98 @@ ] } }, + "/api/v2/auth/refresh": { + "post": { + "tags": [ + "Login" + ], + "summary": "Exchange the refresh token cookie for a new access token", + "description": "Reads the refreshToken cookie set by /api/v2/auth/token, rotates it and answers\n with a new access token. Needs no Authorization header, so it keeps working once the previous\n access token has expired. The rotated cookie is returned in Set-Cookie; the token itself is\n never part of the body.", + "responses": { + "201": { + "description": "Success", + "headers": { + "Set-Cookie": { + "description": "The rotated refresh token, scoped to /api/v2/auth/refresh and marked HttpOnly.\n Replaces the cookie sent with the request, which is consumed by this call.", + "schema": { + "type": "string", + "example": "refreshToken=4fa1371293a112224bc930cf9fecd0fd; Path=/api/v2/auth/refresh; Max-Age=1209600; Expires=Wed, 30 Sep 2026 07:02:31 GMT; HttpOnly; SameSite=Strict" + } + } + }, + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Token" + } + } + } + }, + "401": { + "description": "The refresh token is missing, expired, revoked or already used", + "content": { + "application/problem+json": { + "schema": { + "$ref": "#/components/schemas/ErrorResponse" + } + } + } + }, + "403": { + "description": "The user has been deactivated", + "content": { + "application/problem+json": { + "schema": { + "$ref": "#/components/schemas/ErrorResponse" + } + } + } + } + }, + "security": [ + { + "refreshCookie": [] + } + ] + }, + "delete": { + "tags": [ + "Login" + ], + "summary": "Log out", + "description": "Revokes the session the refreshToken cookie belongs to and clears the cookie.\n Sessions on other devices are left alone. Logging out without a cookie is not an error.", + "responses": { + "204": { + "description": "Success", + "headers": { + "Set-Cookie": { + "description": "The refresh token cookie, emptied and expired so the client drops it. Carries\n the same attributes it was set with, which is what makes a browser replace rather than keep it.", + "schema": { + "type": "string", + "example": "refreshToken=; Path=/api/v2/auth/refresh; Max-Age=0; Expires=Wed, 16 Sep 2026 07:02:31 GMT; HttpOnly; SameSite=Strict" + } + } + } + }, + "403": { + "description": "The request origin is not allowed to send credentials", + "content": { + "application/problem+json": { + "schema": { + "$ref": "#/components/schemas/ErrorResponse" + } + } + } + } + }, + "security": [ + { + "refreshCookie": [] + }, + {} + ] + } + }, "/api/v2/helper/importFile": { "post": { "parameters": [ @@ -3250,6 +3342,12 @@ "type": "http", "description": "Basic Authorization header.", "scheme": "basic" + }, + "refreshCookie": { + "type": "apiKey", + "description": "HttpOnly cookie holding the refresh token, set by /api/v2/auth/token and scoped to /api/v2/auth/refresh.", + "in": "cookie", + "name": "refreshToken" } } } diff --git a/ci/phpunit/fixtures/openapi/hashtype.spec.json b/ci/phpunit/fixtures/openapi/hashtype.spec.json index 9c3f29d22..1ada9bcff 100644 --- a/ci/phpunit/fixtures/openapi/hashtype.spec.json +++ b/ci/phpunit/fixtures/openapi/hashtype.spec.json @@ -752,6 +752,98 @@ ] } }, + "/api/v2/auth/refresh": { + "post": { + "tags": [ + "Login" + ], + "summary": "Exchange the refresh token cookie for a new access token", + "description": "Reads the refreshToken cookie set by /api/v2/auth/token, rotates it and answers\n with a new access token. Needs no Authorization header, so it keeps working once the previous\n access token has expired. The rotated cookie is returned in Set-Cookie; the token itself is\n never part of the body.", + "responses": { + "201": { + "description": "Success", + "headers": { + "Set-Cookie": { + "description": "The rotated refresh token, scoped to /api/v2/auth/refresh and marked HttpOnly.\n Replaces the cookie sent with the request, which is consumed by this call.", + "schema": { + "type": "string", + "example": "refreshToken=4fa1371293a112224bc930cf9fecd0fd; Path=/api/v2/auth/refresh; Max-Age=1209600; Expires=Wed, 30 Sep 2026 07:02:31 GMT; HttpOnly; SameSite=Strict" + } + } + }, + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Token" + } + } + } + }, + "401": { + "description": "The refresh token is missing, expired, revoked or already used", + "content": { + "application/problem+json": { + "schema": { + "$ref": "#/components/schemas/ErrorResponse" + } + } + } + }, + "403": { + "description": "The user has been deactivated", + "content": { + "application/problem+json": { + "schema": { + "$ref": "#/components/schemas/ErrorResponse" + } + } + } + } + }, + "security": [ + { + "refreshCookie": [] + } + ] + }, + "delete": { + "tags": [ + "Login" + ], + "summary": "Log out", + "description": "Revokes the session the refreshToken cookie belongs to and clears the cookie.\n Sessions on other devices are left alone. Logging out without a cookie is not an error.", + "responses": { + "204": { + "description": "Success", + "headers": { + "Set-Cookie": { + "description": "The refresh token cookie, emptied and expired so the client drops it. Carries\n the same attributes it was set with, which is what makes a browser replace rather than keep it.", + "schema": { + "type": "string", + "example": "refreshToken=; Path=/api/v2/auth/refresh; Max-Age=0; Expires=Wed, 16 Sep 2026 07:02:31 GMT; HttpOnly; SameSite=Strict" + } + } + } + }, + "403": { + "description": "The request origin is not allowed to send credentials", + "content": { + "application/problem+json": { + "schema": { + "$ref": "#/components/schemas/ErrorResponse" + } + } + } + } + }, + "security": [ + { + "refreshCookie": [] + }, + {} + ] + } + }, "/api/v2/helper/importFile": { "post": { "parameters": [ @@ -1631,6 +1723,12 @@ "type": "http", "description": "Basic Authorization header.", "scheme": "basic" + }, + "refreshCookie": { + "type": "apiKey", + "description": "HttpOnly cookie holding the refresh token, set by /api/v2/auth/token and scoped to /api/v2/auth/refresh.", + "in": "cookie", + "name": "refreshToken" } } } diff --git a/ci/phpunit/inc/apiv2/auth/JWTBeforeHandlerTest.php b/ci/phpunit/inc/apiv2/auth/JWTBeforeHandlerTest.php new file mode 100644 index 000000000..75b99a624 --- /dev/null +++ b/ci/phpunit/inc/apiv2/auth/JWTBeforeHandlerTest.php @@ -0,0 +1,80 @@ + $decoded + */ + private function handle(array $decoded) { + $request = (new ServerRequestFactory())->createServerRequest('GET', 'http://localhost/api/v2/ui/users'); + + return (new JWTBeforeHandler())($request, ['decoded' => $decoded, 'token' => 'irrelevant']); + } + + private function accessClaims(): array { + return ['userId' => 1, 'scope' => 'ALL', 'aud' => 'user_hashtopolis', 'type' => DTokenType::ACCESS]; + } + + public function testAnAccessTokenIsAccepted(): void { + $request = $this->handle($this->accessClaims()); + + $this->assertSame(1, $request->getAttribute('userId')); + $this->assertSame('ALL', $request->getAttribute('scope')); + } + + /** + * The gate itself: a credential minted for renewing a session must not authorise a resource request. + */ + public function testARefreshTokenIsRefused(): void { + $claims = $this->accessClaims(); + $claims['type'] = DTokenType::REFRESH; + + $this->expectException(HttpForbidden::class); + $this->handle($claims); + } + + public function testAnUnknownTypeIsRefused(): void { + $claims = $this->accessClaims(); + $claims['type'] = 'something-else'; + + $this->expectException(HttpForbidden::class); + $this->handle($claims); + } + + /** + * Tokens minted before the claim existed carry no type and stay usable until they expire, so + * deploying this does not log everyone out. + */ + public function testATokenWithoutATypeClaimIsStillAccepted(): void { + $claims = $this->accessClaims(); + unset($claims['type']); + + $this->assertSame(1, $this->handle($claims)->getAttribute('userId')); + } + + /** + * The audience is carried through for permission checks, and defaults when absent. + */ + public function testAudienceIsExposedToTheRequest(): void { + $this->assertSame('user_hashtopolis', $this->handle($this->accessClaims())->getAttribute('aud')); + + $claims = $this->accessClaims(); + unset($claims['aud']); + $this->assertSame('user_hashtopolis', $this->handle($claims)->getAttribute('aud')); + } +} diff --git a/ci/phpunit/inc/apiv2/util/CorsHackMiddlewareTest.php b/ci/phpunit/inc/apiv2/util/CorsHackMiddlewareTest.php index 57cace8e5..acade9c93 100644 --- a/ci/phpunit/inc/apiv2/util/CorsHackMiddlewareTest.php +++ b/ci/phpunit/inc/apiv2/util/CorsHackMiddlewareTest.php @@ -6,10 +6,14 @@ use Hashtopolis\inc\apiv2\error\HttpForbidden; +use Override; +use PHPUnit\Framework\Attributes\DataProvider; + use Slim\Factory\AppFactory; class DummyRequest { private string $http_origin; + private string $host = 'hashtopolis.example.com'; public function setHeaderLine($headerLine): void { $this->http_origin = $headerLine; @@ -18,9 +22,46 @@ public function setHeaderLine($headerLine): void { public function getHeaderLine($headerLine): string { return $this->http_origin; } + + public function setHost(string $host): void { + $this->host = $host; + } + + /** Enough of a PSR-7 URI for the cross-site guard, which only asks for the host. */ + public function getUri(): object { + return new class($this->host) { + public function __construct(private string $host) {} + + public function getHost(): string { + return $this->host; + } + }; + } } final class CorsHackMiddlewareTest extends TestCase { + /** + * Each test sets only the variables it exercises, so anything left behind by the previous test + * would silently take part in the next one and make results depend on execution order. + */ + #[Override] + protected function setUp(): void { + parent::setUp(); + + putenv("HASHTOPOLIS_BACKEND_URL"); + putenv("HASHTOPOLIS_FRONTEND_PORT"); + putenv("HASHTOPOLIS_FRONTEND_URLS"); + } + + #[Override] + protected function tearDown(): void { + putenv("HASHTOPOLIS_BACKEND_URL"); + putenv("HASHTOPOLIS_FRONTEND_PORT"); + putenv("HASHTOPOLIS_FRONTEND_URLS"); + + parent::tearDown(); + } + /** * Tests all possible valid localhost variations with different ports. * @@ -56,25 +97,32 @@ public function testValidLocalhostVariations(): void { $request->setHeaderLine("http://[::1]:8080"); CorsHackMiddleware::CheckCORS($request, $response); + } - //Test the same but with https: - $request->setHeaderLine("https://127.0.0.1:4200"); - CorsHackMiddleware::CheckCORS($request, $response); + /** + * The same localhost variations over https, against an https backend. + * + * @return void + * @throws HttpForbidden + */ + public function testValidLocalhostVariationsOverHttps(): void { + $this->expectNotToPerformAssertions(); - $request->setHeaderLine("https://localhost:4200"); - CorsHackMiddleware::CheckCORS($request, $response); + putenv("HASHTOPOLIS_BACKEND_URL=https://localhost:8080/api/v2"); + putenv("HASHTOPOLIS_FRONTEND_PORT=4200"); - $request->setHeaderLine("https://[::1]:4200"); - CorsHackMiddleware::CheckCORS($request, $response); + $app = AppFactory::create(); - $request->setHeaderLine("https://127.0.0.1:8080"); - CorsHackMiddleware::CheckCORS($request, $response); + $request = new DummyRequest(); - $request->setHeaderLine("https://localhost:8080"); - CorsHackMiddleware::CheckCORS($request, $response); + $response = $app->getResponseFactory()->createResponse(); - $request->setHeaderLine("https://[::1]:8080"); - CorsHackMiddleware::CheckCORS($request, $response); + foreach (["127.0.0.1", "localhost", "[::1]"] as $host) { + foreach ([4200, 8080] as $port) { + $request->setHeaderLine("https://$host:$port"); + CorsHackMiddleware::CheckCORS($request, $response); + } + } } /** @@ -207,13 +255,16 @@ public function testValidHttpsDomainWithoutPort(): void { } /** - * Tests a valid https-domain with port as origin but configured http-backend-url. - * The http:// or https:// are not part of the CORS checks. + * Tests an https origin against an http backend URL. + * + * The scheme is part of the comparison: http and https are different origins, and treating them + * as one would let a plain-http page read responses meant for the https deployment. A deployment + * that moved behind TLS has to say so in HASHTOPOLIS_BACKEND_URL. * * @throws HttpForbidden */ - public function testValidHttpsDomainWithPortWithHttpConfig(): void { - $this->expectNotToPerformAssertions(); + public function testSchemeMismatchIsRejected(): void { + $this->expectException(HttpForbidden::class); putenv("HASHTOPOLIS_BACKEND_URL=http://hashtopolis-cluster.com:8080/api/v2"); putenv("HASHTOPOLIS_FRONTEND_PORT=4200"); @@ -245,7 +296,386 @@ public function testValidDomainWithoutDifferentFrontendPort(): void { $response = $app->getResponseFactory()->createResponse(); - $request->setHeaderLine("https://hashtopolis-cluster.com:5000"); + $request->setHeaderLine("http://hashtopolis-cluster.com:5000"); CorsHackMiddleware::CheckCORS($request, $response); } + + /** + * Applies the three settings and answers what the middleware decided, so a case reads as one line + * rather than fifteen of setup. + * + * @return string the echoed origin, "*" for the wildcard, or "REJECTED" + */ + private function decide(?string $backendUrl, ?string $frontendUrls, ?string $frontendPort, string $origin): string { + foreach ([ + "HASHTOPOLIS_BACKEND_URL" => $backendUrl, + "HASHTOPOLIS_FRONTEND_URLS" => $frontendUrls, + "HASHTOPOLIS_FRONTEND_PORT" => $frontendPort + ] as $name => $value) { + putenv($value === null ? $name : "$name=$value"); + } + + $request = new DummyRequest(); + $request->setHeaderLine($origin); + + try { + $response = CorsHackMiddleware::CheckCORS($request, AppFactory::create()->getResponseFactory()->createResponse()); + } catch (HttpForbidden) { + return "REJECTED"; + } + + return $response->getHeaderLine("Access-Control-Allow-Origin"); + } + + public static function allowedOriginProvider(): array { + $api = "https://api.example.com"; + $app = "https://app.example.com"; + + return [ + // A frontend on an entirely different host is the case the list exists for + "different host, listed" => [$api, $app, null, $app, $app], + "different host, not listed" => [$api, "https://other.example.com", null, $app, "REJECTED"], + "different scheme and port, listed" => [$api, "http://app.example.com:8081", null, "http://app.example.com:8081", "http://app.example.com:8081"], + + // The union: every source still contributes while a list is set + "backend own origin survives a list" => [$api, $app, null, $api, $api], + "legacy frontend port survives a list" => [$api, $app, "4200", "https://api.example.com:4200", "https://api.example.com:4200"], + + // The list alone is a complete policy + "list without backend url allows" => [null, $app, null, $app, $app], + "list without backend url rejects others" => [null, $app, null, "https://evil.example", "REJECTED"], + "lonely frontend port allows nothing" => [null, $app, "4200", "https://elsewhere.example:4200", "REJECTED"], + + // Empty is the same as unset, which is what Docker Compose produces for a missing variable + "empty backend url with a list" => ["", $app, null, $app, $app], + "empty backend url alone" => ["", null, null, $app, "*"], + "empty list behaves as unset" => [$api, "", "4200", "https://api.example.com:4200", "https://api.example.com:4200"], + "empty frontend port is ignored" => [$api, $app, "", "https://api.example.com:4200", "REJECTED"], + + // Tolerated shapes + "whitespace around entries" => [$api, " $app , https://b.example.com ", null, $app, $app], + "trailing and doubled commas" => [$api, "$app,,", null, $app, $app], + "entry carrying a path" => [$api, "$app/ui", null, $app, $app], + "entry with a trailing slash" => [$api, "$app/", null, $app, $app], + "mixed case entry" => [$api, "HTTPS://APP.Example.COM", null, $app, $app], + "second entry of several" => [$api, "https://a.example.com,$app", null, $app, $app], + + // Nothing about a list may loosen the matching rule + "suffix of a listed host" => [$api, $app, null, "https://app.example.com.evil.test", "REJECTED"], + "substring of a listed host" => [$api, "https://example.com", null, "https://evil-example.com", "REJECTED"], + "subdomain of a listed host" => [$api, "https://example.com", null, "https://sub.example.com", "REJECTED"], + "listed host on another port" => [$api, $app, null, "https://app.example.com:8443", "REJECTED"], + "listed host on its implied port" => [$api, $app, null, "https://app.example.com:443", "https://app.example.com:443"], + "listed host over another scheme" => [$api, $app, null, "http://app.example.com:443", "REJECTED"], + "literal null origin" => [$api, "$app,null", null, "null", "REJECTED"], + "two origins in one header" => [$api, "https://a.example.com,https://b.example.com", null, "https://a.example.com https://b.example.com", "REJECTED"], + + // The loopback spellings name one machine, per entry + "loopback synonym of an entry" => [null, "http://localhost:4200", null, "http://127.0.0.1:4200", "http://127.0.0.1:4200"], + "ipv6 loopback entry" => [null, "http://[::1]:4200", null, "http://[::1]:4200", "http://[::1]:4200"], + "a non loopback address is not a synonym" => [null, "http://127.0.0.2:4200", null, "http://localhost:4200", "REJECTED"], + ]; + } + + /** + * @throws HttpForbidden + */ + #[DataProvider('allowedOriginProvider')] + public function testOriginDecisions(?string $backendUrl, ?string $frontendUrls, ?string $frontendPort, string $origin, string $expected): void { + $this->assertSame($expected, $this->decide($backendUrl, $frontendUrls, $frontendPort, $origin)); + } + + /** + * A listed origin is trusted with credentials; the wildcard never is. + * + * @throws HttpForbidden + */ + public function testListedOriginIsTrustedWithCredentials(): void { + putenv("HASHTOPOLIS_FRONTEND_URLS=https://app.example.com"); + + $request = new DummyRequest(); + $request->setHeaderLine("https://app.example.com"); + + $response = CorsHackMiddleware::CheckCORS($request, AppFactory::create()->getResponseFactory()->createResponse()); + + $this->assertSame("https://app.example.com", $response->getHeaderLine("Access-Control-Allow-Origin")); + $this->assertSame("true", $response->getHeaderLine("Access-Control-Allow-Credentials")); + $this->assertStringContainsString("Origin", $response->getHeaderLine("Vary")); + } + + /** + * A duplicated entry must not produce a comma joined header, which no browser accepts. + * + * @throws HttpForbidden + */ + public function testDuplicateEntriesEchoASingleOrigin(): void { + $this->assertSame( + "https://app.example.com", + $this->decide(null, "https://app.example.com,https://app.example.com", null, "https://app.example.com") + ); + } + + /** + * A list that is set but matches nothing must never fall back to the wildcard: that is the + * fail-open direction, and it would handing out a permissive policy exactly when one was refused. + * + * @throws HttpForbidden + */ + public function testAnUnmatchedListNeverFallsBackToTheWildcard(): void { + $this->assertNotSame("*", $this->decide(null, "https://app.example.com", null, "https://evil.example")); + } + + #[DataProvider('malformedEntryProvider')] + public function testMalformedListEntryIsReported(string $entry): void { + putenv("HASHTOPOLIS_FRONTEND_URLS=$entry"); + + $request = new DummyRequest(); + $request->setHeaderLine("https://app.example.com"); + + try { + CorsHackMiddleware::CheckCORS($request, AppFactory::create()->getResponseFactory()->createResponse()); + $this->fail("A malformed HASHTOPOLIS_FRONTEND_URLS entry should be reported, not ignored"); + } catch (HttpForbidden $e) { + $this->assertStringContainsString($entry, $e->getMessage(), "the message should name the offending entry"); + } + } + + public static function malformedEntryProvider(): array { + return [ + "bare host" => ["app.example.com"], + "bare host and port" => ["app.example.com:4200"], + "bare port" => ["4200"], + "scheme relative" => ["//app.example.com"], + "ftp" => ["ftp://app.example.com"], + "file" => ["file:///etc/passwd"], + "javascript" => ["javascript:alert(1)"], + "the word null" => ["null"], + ]; + } + + /** + * One bad entry invalidates the whole list, including for an origin that matches a good entry + * before it. Otherwise whether a typo is noticed depends on the order of the list. + */ + public function testAMalformedEntryInvalidatesTheWholeList(): void { + $this->expectException(HttpForbidden::class); + + putenv("HASHTOPOLIS_FRONTEND_URLS=https://app.example.com,not-an-origin"); + + $request = new DummyRequest(); + $request->setHeaderLine("https://app.example.com"); + + CorsHackMiddleware::CheckCORS($request, AppFactory::create()->getResponseFactory()->createResponse()); + } + + /** + * A CR/LF bearing origin must not match and must not reach the response headers. + * + * @throws HttpForbidden + */ + public function testOriginWithControlCharactersIsRejected(): void { + putenv("HASHTOPOLIS_FRONTEND_URLS=https://app.example.com"); + + $request = new DummyRequest(); + $request->setHeaderLine("https://app.example.com\r\nX-Evil: 1"); + + $response = AppFactory::create()->getResponseFactory()->createResponse(); + + try { + $response = CorsHackMiddleware::CheckCORS($request, $response); + } catch (HttpForbidden) { + $this->addToAssertionCount(1); + } + + $this->assertFalse($response->hasHeader("X-Evil")); + } + + /** + * The cross-site guard protects endpoints that authenticate from a cookie. A browser attaches the + * cookie to whatever request a page makes, so without this a page elsewhere could have a visitor's + * browser spend their refresh token or end their session. + * + * @throws HttpForbidden + */ + public function testCrossSiteGuardAcceptsAConfiguredOrigin(): void { + putenv("HASHTOPOLIS_BACKEND_URL=https://hashtopolis.example.com"); + putenv("HASHTOPOLIS_FRONTEND_URLS=https://app.example.com"); + + $request = new DummyRequest(); + $request->setHeaderLine("https://app.example.com"); + + CorsHackMiddleware::assertNotCrossSite($request); + $this->addToAssertionCount(1); + } + + public function testCrossSiteGuardRefusesAnUnconfiguredOrigin(): void { + putenv("HASHTOPOLIS_BACKEND_URL=https://hashtopolis.example.com"); + putenv("HASHTOPOLIS_FRONTEND_URLS=https://app.example.com"); + + $request = new DummyRequest(); + $request->setHeaderLine("https://evil.example"); + + $this->expectException(HttpForbidden::class); + CorsHackMiddleware::assertNotCrossSite($request); + } + + /** + * A client that sends no origin is not a browser, so it holds no ambient cookie to be abused and + * must keep working: this is how curl and the python client call the API. + * + * @throws HttpForbidden + */ + public function testCrossSiteGuardAllowsARequestWithoutAnOrigin(): void { + putenv("HASHTOPOLIS_BACKEND_URL=https://hashtopolis.example.com"); + + $request = new DummyRequest(); + $request->setHeaderLine(""); + + CorsHackMiddleware::assertNotCrossSite($request); + $this->addToAssertionCount(1); + } + + /** + * With no allow-list there is nothing to compare an origin against, and the CORS layer answers the + * wildcard. A cookie endpoint cannot rely on that, so the guard falls back to the host the request + * was addressed to. + * + * @throws HttpForbidden + */ + public function testCrossSiteGuardFallsBackToTheRequestHost(): void { + $request = new DummyRequest(); + $request->setHost("hashtopolis.example.com"); + $request->setHeaderLine("https://hashtopolis.example.com"); + + CorsHackMiddleware::assertNotCrossSite($request); + $this->addToAssertionCount(1); + } + + /** + * The case SameSite=Strict does not cover: a neighbouring host is same-site, so the browser sends + * the cookie, but it is a different origin and has no business acting for the user. + */ + public function testCrossSiteGuardRefusesANeighbouringHostWithNoAllowList(): void { + $request = new DummyRequest(); + $request->setHost("hashtopolis.example.com"); + $request->setHeaderLine("https://evil.hashtopolis.example.com"); + + $this->expectException(HttpForbidden::class); + CorsHackMiddleware::assertNotCrossSite($request); + } + + /** + * Regression: a portless origin must not match a portless HASHTOPOLIS_BACKEND_URL. + * + * Slicing each URL at its last colon turned both a portless origin and a portless backend URL into + * an empty host, which compared equal. Any site could therefore be echoed back as an allowed + * origin, and since these responses also carry Allow-Credentials, read authenticated replies. The + * configuration that triggered it is the ordinary one for TLS: a backend URL with no explicit port. + * + * @throws HttpForbidden + */ + public function testPortlessEvilOriginDoesNotMatchPortlessBackendUrl(): void { + $this->expectException(HttpForbidden::class); + + putenv("HASHTOPOLIS_BACKEND_URL=https://hashtopolis-cluster.com/api/v2"); + putenv("HASHTOPOLIS_FRONTEND_PORT=4200"); + + $app = AppFactory::create(); + + $request = new DummyRequest(); + + $response = $app->getResponseFactory()->createResponse(); + + $request->setHeaderLine("https://evil.com"); + CorsHackMiddleware::CheckCORS($request, $response); + } + + /** + * A subdomain is a different origin, even though it shares a suffix with the configured host. + * + * @throws HttpForbidden + */ + public function testSubdomainOfConfiguredHostIsRejected(): void { + $this->expectException(HttpForbidden::class); + + putenv("HASHTOPOLIS_BACKEND_URL=https://hashtopolis-cluster.com/api/v2"); + putenv("HASHTOPOLIS_FRONTEND_PORT=4200"); + + $app = AppFactory::create(); + + $request = new DummyRequest(); + + $response = $app->getResponseFactory()->createResponse(); + + $request->setHeaderLine("https://evil.hashtopolis-cluster.com"); + CorsHackMiddleware::CheckCORS($request, $response); + } + + /** + * The port a scheme implies is filled in, so the two spellings of the same origin agree. + * + * @throws HttpForbidden + */ + public function testExplicitDefaultPortMatchesPortlessBackendUrl(): void { + putenv("HASHTOPOLIS_BACKEND_URL=https://hashtopolis-cluster.com/api/v2"); + putenv("HASHTOPOLIS_FRONTEND_PORT=4200"); + + $app = AppFactory::create(); + + $request = new DummyRequest(); + + $response = $app->getResponseFactory()->createResponse(); + + $request->setHeaderLine("https://hashtopolis-cluster.com:443"); + $response = CorsHackMiddleware::CheckCORS($request, $response); + + $this->assertSame("https://hashtopolis-cluster.com:443", $response->getHeaderLine("Access-Control-Allow-Origin")); + } + + /** + * A verified origin is the only thing credentials are granted to, and the response says it varies + * by origin so a cache cannot hand one origin's headers to another. + * + * @throws HttpForbidden + */ + public function testVerifiedOriginGetsCredentialsAndVaries(): void { + putenv("HASHTOPOLIS_BACKEND_URL=http://localhost:8080/api/v2"); + putenv("HASHTOPOLIS_FRONTEND_PORT=4200"); + + $app = AppFactory::create(); + + $request = new DummyRequest(); + + $response = $app->getResponseFactory()->createResponse(); + + $request->setHeaderLine("http://localhost:4200"); + $response = CorsHackMiddleware::CheckCORS($request, $response); + + $this->assertSame("http://localhost:4200", $response->getHeaderLine("Access-Control-Allow-Origin")); + $this->assertSame("true", $response->getHeaderLine("Access-Control-Allow-Credentials")); + $this->assertStringContainsString("Origin", $response->getHeaderLine("Vary")); + } + + /** + * Without a configured backend URL there is nothing to verify an origin against, so the API stays + * open but never grants credentials: browsers refuse them next to a wildcard. + * + * @throws HttpForbidden + */ + public function testWithoutBackendUrlTheWildcardCarriesNoCredentials(): void { + putenv("HASHTOPOLIS_BACKEND_URL"); + putenv("HASHTOPOLIS_FRONTEND_PORT"); + + $app = AppFactory::create(); + + $request = new DummyRequest(); + + $response = $app->getResponseFactory()->createResponse(); + + $request->setHeaderLine("https://evil.com"); + $response = CorsHackMiddleware::CheckCORS($request, $response); + + $this->assertSame("*", $response->getHeaderLine("Access-Control-Allow-Origin")); + $this->assertSame("", $response->getHeaderLine("Access-Control-Allow-Credentials")); + } } diff --git a/ci/phpunit/inc/utils/RefreshTokenUtilsTest.php b/ci/phpunit/inc/utils/RefreshTokenUtilsTest.php new file mode 100644 index 000000000..dd40b00b2 --- /dev/null +++ b/ci/phpunit/inc/utils/RefreshTokenUtilsTest.php @@ -0,0 +1,256 @@ +user = $this->createUser('rt_user'); + } + + /** + * Looks a token up the way the production code does, so the test never assumes a row id. + */ + private function findToken(string $plain): ?RefreshToken { + $qF = new QueryFilter(RefreshToken::TOKEN_HASH, RefreshTokenUtils::hashToken($plain), "="); + return Factory::getRefreshTokenFactory()->filter([Factory::FILTER => $qF], true); + } + + /** + * @return RefreshToken[] + */ + private function tokensOfUser(): array { + $qF = new QueryFilter(RefreshToken::USER_ID, $this->user->getId(), "="); + return Factory::getRefreshTokenFactory()->filter([Factory::FILTER => $qF]); + } + + public function testIssueStoresOnlyTheHashOfTheToken(): void { + $plain = RefreshTokenUtils::issue($this->user->getId()); + + $token = $this->findToken($plain); + $this->assertInstanceOf(RefreshToken::class, $token); + $this->assertNotSame($plain, $token->getTokenHash()); + $this->assertSame(hash('sha256', $plain), $token->getTokenHash()); + $this->assertNull($token->getUsedAt()); + $this->assertEquals(0, $token->getIsRevoked()); + } + + public function testIssueStartsANewFamilyPerLogin(): void { + $first = $this->findToken(RefreshTokenUtils::issue($this->user->getId())); + $second = $this->findToken(RefreshTokenUtils::issue($this->user->getId())); + + $this->assertNotSame($first->getFamilyId(), $second->getFamilyId()); + } + + public function testRotateConsumesTheTokenAndKeepsTheFamily(): void { + $plain = RefreshTokenUtils::issue($this->user->getId()); + $original = $this->findToken($plain); + + $rotated = RefreshTokenUtils::rotate($plain); + + $this->assertSame($this->user->getId(), $rotated['user']->getId()); + $this->assertNotSame($plain, $rotated['token']); + + $consumed = $this->findToken($plain); + $this->assertNotNull($consumed->getUsedAt()); + + $successor = $this->findToken($rotated['token']); + $this->assertNull($successor->getUsedAt()); + $this->assertSame($original->getFamilyId(), $successor->getFamilyId()); + } + + public function testRotateRejectsAnUnknownToken(): void { + $this->expectException(HttpUnauthorized::class); + RefreshTokenUtils::rotate('not-a-token'); + } + + public function testRotateRejectsAnExpiredToken(): void { + $plain = RefreshTokenUtils::issue($this->user->getId()); + Factory::getRefreshTokenFactory()->set($this->findToken($plain), RefreshToken::END_VALID, time() - 1); + + $this->expectException(HttpUnauthorized::class); + RefreshTokenUtils::rotate($plain); + } + + public function testReplayRevokesTheWholeFamily(): void { + $plain = RefreshTokenUtils::issue($this->user->getId()); + $rotated = RefreshTokenUtils::rotate($plain); + + try { + RefreshTokenUtils::rotate($plain); + $this->fail('Replaying a consumed refresh token should not hand out a new one'); + } catch (HttpUnauthorized $e) { + $this->assertStringContainsString('already been used', $e->getMessage()); + } + + // The successor was never touched by the attacker, but the session is gone all the same + $this->assertEquals(1, $this->findToken($rotated['token'])->getIsRevoked()); + + $this->expectException(HttpUnauthorized::class); + RefreshTokenUtils::rotate($rotated['token']); + } + + public function testConsumingATokenIsAnAtomicTestAndSet(): void { + /* The mechanism replay detection rests on: consuming a token has to be the same statement that + checks it is unconsumed, so that concurrent requests cannot both find it unused. Running the + claim twice stands in for two requests arriving together; only one may report having made the + transition. */ + $plain = RefreshTokenUtils::issue($this->user->getId()); + $token = $this->findToken($plain); + + $claim = fn(): bool => Factory::getRefreshTokenFactory()->compareAndSet( + $token, + [RefreshToken::USED_AT => null, RefreshToken::IS_REVOKED => 0], + [RefreshToken::USED_AT => time()] + ); + + $this->assertTrue($claim(), 'the first caller should win the token'); + $this->assertFalse($claim(), 'a second caller must not also win the same token'); + } + + public function testLosingTheClaimLeavesTheWinnersTimestampAlone(): void { + $plain = RefreshTokenUtils::issue($this->user->getId()); + $token = $this->findToken($plain); + + // Stand in for a request that already consumed the token a moment ago + $consumedAt = time() - 1; + Factory::getRefreshTokenFactory()->set($token, RefreshToken::USED_AT, $consumedAt); + + try { + RefreshTokenUtils::rotate($plain); + $this->fail('A consumed token should not be exchangeable'); + } catch (HttpUnauthorized) { + // The conditional update has to leave a row it did not match untouched, so the record of when + // the token was really consumed survives for anyone investigating the replay + $this->assertSame($consumedAt, (int)$this->findToken($plain)->getUsedAt()); + } + } + + public function testARevokedTokenCannotBeClaimed(): void { + $plain = RefreshTokenUtils::issue($this->user->getId()); + RefreshTokenUtils::revoke($plain); + + try { + RefreshTokenUtils::rotate($plain); + $this->fail('A revoked refresh token should not be exchangeable'); + } catch (HttpUnauthorized $e) { + $this->assertNull($this->findToken($plain)->getUsedAt(), 'a revoked token should not be consumed'); + } + } + + /** + * A token is single use with no window of forgiveness. An immediate second exchange is exactly what + * a copied cookie looks like, and it cannot be told apart from a client that asked twice, so it is + * treated as the leak it might be. Clients avoid this by serialising their own refreshes. + */ + public function testASecondExchangeIsAReplayEvenImmediately(): void { + $plain = RefreshTokenUtils::issue($this->user->getId()); + + $first = RefreshTokenUtils::rotate($plain); + + try { + RefreshTokenUtils::rotate($plain); + $this->fail('A refresh token must not be exchangeable twice, however quickly the second use arrives'); + } catch (HttpUnauthorized $e) { + $this->assertStringContainsString('already been used', $e->getMessage()); + } + + // ... and the successor the first exchange handed out goes with it + $this->assertEquals(1, $this->findToken($first['token'])->getIsRevoked()); + } + + public function testRotateRefusesADeactivatedUser(): void { + $plain = RefreshTokenUtils::issue($this->user->getId()); + Factory::getUserFactory()->set($this->user, User::IS_VALID, 0); + + try { + RefreshTokenUtils::rotate($plain); + $this->fail('A deactivated user should not be able to refresh'); + } catch (HttpForbidden $e) { + $this->assertEquals(1, $this->findToken($plain)->getIsRevoked()); + } finally { + Factory::getUserFactory()->set($this->user, User::IS_VALID, 1); + } + } + + public function testRevokeEndsOnlyItsOwnSession(): void { + $sessionA = RefreshTokenUtils::issue($this->user->getId()); + $sessionB = RefreshTokenUtils::issue($this->user->getId()); + + RefreshTokenUtils::revoke($sessionA); + + $this->assertEquals(1, $this->findToken($sessionA)->getIsRevoked()); + $this->assertEquals(0, $this->findToken($sessionB)->getIsRevoked()); + } + + public function testRevokeIgnoresAnUnknownToken(): void { + RefreshTokenUtils::revoke('not-a-token'); + $this->assertCount(0, $this->tokensOfUser()); + } + + public function testRevokeAllForUserEndsEverySession(): void { + $sessionA = RefreshTokenUtils::issue($this->user->getId()); + $sessionB = RefreshTokenUtils::issue($this->user->getId()); + + RefreshTokenUtils::revokeAllForUser($this->user->getId()); + + $this->assertEquals(1, $this->findToken($sessionA)->getIsRevoked()); + $this->assertEquals(1, $this->findToken($sessionB)->getIsRevoked()); + } + + public function testPurgeExpiredKeepsTokensWhichCanStillBeExchanged(): void { + $live = RefreshTokenUtils::issue($this->user->getId()); + $stale = RefreshTokenUtils::issue($this->user->getId()); + Factory::getRefreshTokenFactory()->set($this->findToken($stale), RefreshToken::END_VALID, time() - 1); + + RefreshTokenUtils::purgeExpired($this->user->getId()); + + $this->assertNull($this->findToken($stale)); + $this->assertNotNull($this->findToken($live)); + } + + public function testDeleteAllForUserLeavesNothingBehind(): void { + RefreshTokenUtils::issue($this->user->getId()); + RefreshTokenUtils::issue($this->user->getId()); + + RefreshTokenUtils::deleteAllForUser($this->user->getId()); + + $this->assertCount(0, $this->tokensOfUser()); + } + + public function testDeletingAUserRemovesItsRefreshTokens(): void { + // Deliberately not registered for teardown: the test deletes the user itself + $name = 'rt_victim_' . uniqid(); + $victim = UserUtils::createUser($name, $name . '@example.com', $this->createRightGroup()->getId(), $this->adminUser); + $plain = RefreshTokenUtils::issue($victim->getId()); + + UserUtils::deleteUser($victim->getId(), $this->adminUser); + + $this->assertNull($this->findToken($plain)); + } + + public function testDisablingAUserRevokesItsRefreshTokens(): void { + $plain = RefreshTokenUtils::issue($this->user->getId()); + + UserUtils::disableUser($this->user->getId(), $this->adminUser); + + $this->assertEquals(1, $this->findToken($plain)->getIsRevoked()); + UserUtils::enableUser($this->user->getId()); + } +} diff --git a/doc/faq_tips/faq.md b/doc/faq_tips/faq.md index 2ddd4084e..e734d626f 100644 --- a/doc/faq_tips/faq.md +++ b/doc/faq_tips/faq.md @@ -428,6 +428,25 @@ Zaps are notification to agents that another agent already cracked a hash, allow --- +❓ The browser console shows a CORS error, or I get logged out every couple of hours + +Both point at the same setting. The API only shares responses with origins it has been told to trust, +and only sends the session cookie to those origins. If the frontend is served from a different +hostname or port than the API, it has to be named — see +[Serving the frontend on another origin](../installation_guidelines/advanced_install.md#serving-the-frontend-on-another-origin). + +Being logged out roughly every two hours with no visible error is the subtler half of the same +problem: logging in works, but the session cannot be renewed because the cookie never reaches the +API, so it lasts exactly as long as the access token. + +The backend logs the origin it rejected together with the list it would have accepted: + +``` +docker logs hashtopolis-backend +``` + +--- + ## Security & Access Control ❓ Is there a way to trust all agents by default? diff --git a/doc/installation_guidelines/advanced_install.md b/doc/installation_guidelines/advanced_install.md index dd462c28a..c0a718ba9 100644 --- a/doc/installation_guidelines/advanced_install.md +++ b/doc/installation_guidelines/advanced_install.md @@ -257,6 +257,86 @@ docker compose up Finally, copy the data back into the appropriate folders after recreating the containers. +## Serving the frontend on another origin + +The browser treats `https://hashtopolis.example.com` and `https://app.example.com:4200` as different +*origins*. When the frontend is served from a different origin than the API, the API has to say which +origins it trusts, otherwise the browser refuses to hand the frontend the response. + +This matters beyond the odd console error: the login session lives in a cookie the API sets, and a +browser will not send that cookie cross-origin unless the API names the frontend's origin +specifically. Get it wrong and the symptom is not an error at all — people log in fine and are +silently logged out a couple of hours later, when the short-lived access token expires and the +session cannot be renewed. + +### Both served from one origin: nothing to configure + +If a reverse proxy serves the frontend at `/` and the API under `/api` on the same hostname (see +[SSL/TLS Setup](tls.md)), the browser sees a single origin, none of this applies, and you can leave +all of the settings below unset. This is the simplest deployment and the one to prefer. + +### Frontend on a different origin + +Set these on the **backend** container: + +| Variable | Meaning | +| --- | --- | +| `HASHTOPOLIS_BACKEND_URL` | the API's own URL, for example `https://hashtopolis.example.com/api/v2`. The API always trusts its own origin. | +| `HASHTOPOLIS_FRONTEND_URLS` | extra origins to trust, comma separated. Use this when the frontend is on a different **host**. | +| `HASHTOPOLIS_FRONTEND_PORT` | shorthand for "the API's own host and scheme, on this port". Enough when the frontend differs only by **port**, which is what the bundled `docker-compose.yml` does with port 4200. | + +An origin is trusted when it matches one of these exactly — scheme, host and port all three. There +are no wildcards: `https://*.example.com` is not a pattern, and `https://app.example.com` does not +admit `https://app.example.com.somewhere-else.test`. A trusted origin is handed the session cookie, +so list only origins you control. + +A frontend at `https://app.example.com` talking to an API at `https://hashtopolis.example.com`: + +``` +HASHTOPOLIS_BACKEND_URL=https://hashtopolis.example.com/api/v2 +HASHTOPOLIS_FRONTEND_URLS=https://app.example.com +``` + +Entries may carry a path, which is ignored — only the origin part is compared. `localhost`, +`127.0.0.1` and `[::1]` are treated as the same host, so a development setup can mix them freely. + +If you have enumerated every origin in `HASHTOPOLIS_FRONTEND_URLS` and do not want the bundled +`HASHTOPOLIS_FRONTEND_PORT: 4200` adding one more, set it to an empty string to switch it off. + +> [!WARNING] +> Set `HASHTOPOLIS_BACKEND_URL` together with one of the other two, or set none of them. On its own +> it tells the API to trust exactly one origin — its own — and every request from the frontend is +> then rejected with a CORS error. + +### When the frontend is on a different domain entirely + +`app.example.com` and `hashtopolis.example.com` are different origins but the same *site*, and the +session cookie works across them with no further configuration. + +A frontend on a genuinely different domain — `app.example.net` against an API at +`hashtopolis.example.com` — is a different case. Naming it in `HASHTOPOLIS_FRONTEND_URLS` gets the +requests through, but the browser still refuses to send the session cookie, because the cookie +defaults to `SameSite=Strict`. Such a deployment additionally needs +`HASHTOPOLIS_REFRESH_COOKIE_SAMESITE=None` on the backend container, which browsers only honour over +HTTPS end to end. + +### Upgrading from an older version + +`HASHTOPOLIS_FRONTEND_URLS` existed in earlier releases, stopped being read for a while, and is read +again from this release on. Its meaning has changed: it used to be the entire policy, and it now +*adds to* the origins named by `HASHTOPOLIS_BACKEND_URL` and `HASHTOPOLIS_FRONTEND_PORT`. + +If your `.env` still carries a value from back then, review it before upgrading. Anything listed is +trusted with the session cookie, and a hostname you no longer control should not be on that list. + +### When it does not work + +The API logs the origin it rejected and the full list it would have accepted. Look there first: + +``` +docker logs hashtopolis-backend +``` + ## Backup and Restore The best way to back up and restore your Hashtopolis instance depends heavily on the way the instance is set up and what configurations are made. diff --git a/doc/installation_guidelines/tls.md b/doc/installation_guidelines/tls.md index eb3886526..9ea8f6774 100644 --- a/doc/installation_guidelines/tls.md +++ b/doc/installation_guidelines/tls.md @@ -100,6 +100,10 @@ http { 3. Update the value of `HASHTOPOLIS_BACKEND_URL` in the `.env` file to reflect the changes done above. + Serving both through the one proxy as shown above puts the frontend and the API on the same + origin, so no further configuration is needed. If you instead serve the frontend from its own + hostname, see [Serving the frontend on another origin](advanced_install.md#serving-the-frontend-on-another-origin). + 4. Start the containers ``` diff --git a/docker-compose.mysql.yml b/docker-compose.mysql.yml index 0f005ba9f..0e98167ca 100644 --- a/docker-compose.mysql.yml +++ b/docker-compose.mysql.yml @@ -16,6 +16,12 @@ services: HASHTOPOLIS_ADMIN_USER: $HASHTOPOLIS_ADMIN_USER HASHTOPOLIS_ADMIN_PASSWORD: $HASHTOPOLIS_ADMIN_PASSWORD HASHTOPOLIS_BACKEND_URL: $HASHTOPOLIS_BACKEND_URL + # Origins the API shares authenticated responses with, beyond its own. Needed when the frontend + # is served from a different host; see env.*.example. Same-origin setups need neither this nor + # HASHTOPOLIS_FRONTEND_PORT. + HASHTOPOLIS_FRONTEND_URLS: $HASHTOPOLIS_FRONTEND_URLS + # Shorthand for "the backend's own host and scheme, on this port". Set it to "" to switch it + # off and let HASHTOPOLIS_FRONTEND_URLS be the whole story. HASHTOPOLIS_FRONTEND_PORT: 4200 depends_on: - db diff --git a/docker-compose.postgres.yml b/docker-compose.postgres.yml index 151f50b33..2a43e1105 100644 --- a/docker-compose.postgres.yml +++ b/docker-compose.postgres.yml @@ -16,6 +16,12 @@ services: HASHTOPOLIS_ADMIN_USER: $HASHTOPOLIS_ADMIN_USER HASHTOPOLIS_ADMIN_PASSWORD: $HASHTOPOLIS_ADMIN_PASSWORD HASHTOPOLIS_BACKEND_URL: $HASHTOPOLIS_BACKEND_URL + # Origins the API shares authenticated responses with, beyond its own. Needed when the frontend + # is served from a different host; see env.*.example. Same-origin setups need neither this nor + # HASHTOPOLIS_FRONTEND_PORT. + HASHTOPOLIS_FRONTEND_URLS: $HASHTOPOLIS_FRONTEND_URLS + # Shorthand for "the backend's own host and scheme, on this port". Set it to "" to switch it + # off and let HASHTOPOLIS_FRONTEND_URLS be the whole story. HASHTOPOLIS_FRONTEND_PORT: 4200 depends_on: - db diff --git a/env.mysql.example b/env.mysql.example index c1b394af7..93aa774ae 100644 --- a/env.mysql.example +++ b/env.mysql.example @@ -8,3 +8,10 @@ HASHTOPOLIS_ADMIN_PASSWORD=hashtopolis HASHTOPOLIS_DB_HOST=db HASHTOPOLIS_BACKEND_URL=http://localhost:8080/api/v2 + +# Extra origins the API shares authenticated responses with. Only needed when the frontend is served +# from a different host than the API; a reverse proxy that serves both from one origin needs nothing +# here, and a frontend that differs only by port is already covered by HASHTOPOLIS_FRONTEND_PORT. +# Comma separated, one exact origin each: scheme, host and port must match, and there are no +# wildcards. Anything listed is trusted with the session cookie, so keep it to origins you control. +#HASHTOPOLIS_FRONTEND_URLS=https://app.example.com diff --git a/env.postgres.example b/env.postgres.example index 747ce18b7..d1cdf8738 100644 --- a/env.postgres.example +++ b/env.postgres.example @@ -7,3 +7,10 @@ HASHTOPOLIS_ADMIN_PASSWORD=hashtopolis HASHTOPOLIS_DB_HOST=db HASHTOPOLIS_BACKEND_URL=http://localhost:8080/api/v2 + +# Extra origins the API shares authenticated responses with. Only needed when the frontend is served +# from a different host than the API; a reverse proxy that serves both from one origin needs nothing +# here, and a frontend that differs only by port is already covered by HASHTOPOLIS_FRONTEND_PORT. +# Comma separated, one exact origin each: scheme, host and port must match, and there are no +# wildcards. Anything listed is trusted with the session cookie, so keep it to origins you control. +#HASHTOPOLIS_FRONTEND_URLS=https://app.example.com diff --git a/openapi.json b/openapi.json index 77f164a11..c644e4e76 100644 --- a/openapi.json +++ b/openapi.json @@ -28910,6 +28910,100 @@ "operationId": "postToken", "description": "Create Login" } + }, + "/api/v2/auth/refresh": { + "post": { + "tags": [ + "Login" + ], + "summary": "Exchange the refresh token cookie for a new access token", + "description": "Reads the refreshToken cookie set by /api/v2/auth/token, rotates it and answers\n with a new access token. Needs no Authorization header, so it keeps working once the previous\n access token has expired. The rotated cookie is returned in Set-Cookie; the token itself is\n never part of the body.", + "responses": { + "201": { + "description": "Success", + "headers": { + "Set-Cookie": { + "description": "The rotated refresh token, scoped to /api/v2/auth/refresh and marked HttpOnly.\n Replaces the cookie sent with the request, which is consumed by this call.", + "schema": { + "type": "string", + "example": "refreshToken=4fa1371293a112224bc930cf9fecd0fd; Path=/api/v2/auth/refresh; Max-Age=1209600; Expires=Wed, 30 Sep 2026 07:02:31 GMT; HttpOnly; SameSite=Strict" + } + } + }, + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Token" + } + } + } + }, + "401": { + "description": "The refresh token is missing, expired, revoked or already used", + "content": { + "application/problem+json": { + "schema": { + "$ref": "#/components/schemas/ErrorResponse" + } + } + } + }, + "403": { + "description": "The user has been deactivated", + "content": { + "application/problem+json": { + "schema": { + "$ref": "#/components/schemas/ErrorResponse" + } + } + } + } + }, + "security": [ + { + "refreshCookie": [] + } + ], + "operationId": "postRefresh" + }, + "delete": { + "tags": [ + "Login" + ], + "summary": "Log out", + "description": "Revokes the session the refreshToken cookie belongs to and clears the cookie.\n Sessions on other devices are left alone. Logging out without a cookie is not an error.", + "responses": { + "204": { + "description": "Success", + "headers": { + "Set-Cookie": { + "description": "The refresh token cookie, emptied and expired so the client drops it. Carries\n the same attributes it was set with, which is what makes a browser replace rather than keep it.", + "schema": { + "type": "string", + "example": "refreshToken=; Path=/api/v2/auth/refresh; Max-Age=0; Expires=Wed, 16 Sep 2026 07:02:31 GMT; HttpOnly; SameSite=Strict" + } + } + } + }, + "403": { + "description": "The request origin is not allowed to send credentials", + "content": { + "application/problem+json": { + "schema": { + "$ref": "#/components/schemas/ErrorResponse" + } + } + } + } + }, + "security": [ + { + "refreshCookie": [] + }, + {} + ], + "operationId": "deleteRefresh" + } } }, "components": { @@ -72884,6 +72978,12 @@ "type": "http", "description": "Basic Authorization header.", "scheme": "basic" + }, + "refreshCookie": { + "type": "apiKey", + "description": "HttpOnly cookie holding the refresh token, set by /api/v2/auth/token and scoped to /api/v2/auth/refresh.", + "in": "cookie", + "name": "refreshToken" } } }, diff --git a/src/api/v2/index.php b/src/api/v2/index.php index 055215965..e00b3a883 100644 --- a/src/api/v2/index.php +++ b/src/api/v2/index.php @@ -85,7 +85,7 @@ ); $rules = [ - new RequestPathRule(ignore: ["/api/v2/auth/token", "/api/v2/auth/oauth-token", "/api/v2/helper/resetUserPassword", "/api/v2/openapi.json"]), + new RequestPathRule(ignore: ["/api/v2/auth/token", "/api/v2/auth/oauth-token", "/api/v2/auth/refresh", "/api/v2/helper/resetUserPassword", "/api/v2/openapi.json"]), new RequestMethodRule(ignore: ["OPTIONS"]) ]; return new JwtAuthentication($options, $decoder, $rules); @@ -128,7 +128,15 @@ bool $logErrorDetails) use ($app) { $response = $app->getResponseFactory()->createResponse(); - $response = CorsHackMiddleware::addCORSheaders($request, $response); + /* An unrecognised Origin is itself reported by throwing, so decorating the error response can throw + a second time. Answering the original error without CORS headers is the right outcome: the browser + withholds the response from the caller, which is what a rejected origin should get. */ + try { + $response = CorsHackMiddleware::addCORSheaders($request, $response); + } + catch (Throwable $corsException) { + error_log($corsException->getMessage()); + } //Quirk to handle HTExceptions without status code, this can be removed when all HTExceptions have been migrated error_log($exception->getMessage()); diff --git a/src/dba/AbstractModelFactory.php b/src/dba/AbstractModelFactory.php index 9173f3125..6ba515115 100755 --- a/src/dba/AbstractModelFactory.php +++ b/src/dba/AbstractModelFactory.php @@ -330,6 +330,69 @@ public function set(AbstractModel $model, string $key, $value): AbstractModel { return $model; } + /** + * Updates one row, but only while the columns named in $expected still hold the values given. + * + * The test and the write are a single statement, so callers arriving together cannot all read the + * old state and all act on it: the database applies exactly one of them, and the return value says + * which caller that was. Reach for this wherever a transition has to happen at most once - a token + * being consumed, a queued job being picked up, a one-shot flag being flipped. A plain + * "UPDATE ... WHERE id = ?" does not do this: without the expected state in the WHERE clause every + * caller matches the row and every caller wins. + * + * The update has to genuinely change the row. MySQL counts changed rows rather than matched ones, + * so writing a value a column already holds looks identical to not matching at all, and the same + * call would answer differently on PostgreSQL. A transition that writes back what it expected is + * rejected outright rather than being left to differ between the two. + * + * @param TModel $model the row to update, addressed by its primary key + * @param array $expected column => the value the row must still hold, null for IS NULL + * @param array $updates column => the value to write, at least one of them new + * @return bool true when this caller made the transition, false when the row no longer matched + * @throws Exception + */ + public function compareAndSet(AbstractModel $model, array $expected, array $updates): bool { + if (count($updates) == 0) { + throw new Exception("Cannot compare-and-set without any column to update!"); + } + + $changes = array_filter($updates, fn($value, $key) => !array_key_exists($key, $expected) || $expected[$key] !== $value, ARRAY_FILTER_USE_BOTH); + if (count($changes) == 0) { + throw new Exception("Cannot compare-and-set a row to the values it is expected to already hold; the update would change nothing and the result would differ between database types!"); + } + + $values = []; + $assignments = []; + foreach ($updates as $key => $value) { + $assignments[] = self::getMappedModelKey($model, $key) . "=" . self::binaryPlaceholder($model, $key); + $values[] = $value; + } + + // Addressing the row by its primary key is what keeps this a single-row operation + $conditions = [self::getMappedModelKey($model, $model->getPrimaryKey()) . "=?"]; + $values[] = $model->getPrimaryKeyValue(); + + foreach ($expected as $key => $value) { + $column = self::getMappedModelKey($model, $key); + if ($value === null) { + // A parameter compared with = never matches NULL, so the condition has to be spelled out + $conditions[] = $column . " IS NULL"; + continue; + } + $conditions[] = $column . "=" . self::binaryPlaceholder($model, $key); + $values[] = $value; + } + + $query = "UPDATE " . $this->getMappedModelTable() . + " SET " . implode(", ", $assignments) . + " WHERE " . implode(" AND ", $conditions); + + $stmt = $this->getDB()->prepare($query); + $stmt->execute($values); + + return $stmt->rowCount() === 1; + } + /** * Increments the given key of this model by the given value atomically * diff --git a/src/dba/Factory.php b/src/dba/Factory.php index 04e10ffa2..d17b9c337 100644 --- a/src/dba/Factory.php +++ b/src/dba/Factory.php @@ -30,6 +30,7 @@ use Hashtopolis\dba\models\NotificationSettingFactory; use Hashtopolis\dba\models\PreprocessorFactory; use Hashtopolis\dba\models\PretaskFactory; +use Hashtopolis\dba\models\RefreshTokenFactory; use Hashtopolis\dba\models\RegVoucherFactory; use Hashtopolis\dba\models\RightGroupFactory; use Hashtopolis\dba\models\SessionFactory; @@ -79,6 +80,7 @@ class Factory { private static ?NotificationSettingFactory $notificationSettingFactory = null; private static ?PreprocessorFactory $preprocessorFactory = null; private static ?PretaskFactory $pretaskFactory = null; + private static ?RefreshTokenFactory $refreshTokenFactory = null; private static ?RegVoucherFactory $regVoucherFactory = null; private static ?RightGroupFactory $rightGroupFactory = null; private static ?SessionFactory $sessionFactory = null; @@ -379,6 +381,16 @@ public static function getPretaskFactory(): PretaskFactory { } } + public static function getRefreshTokenFactory(): RefreshTokenFactory { + if (self::$refreshTokenFactory == null) { + $f = new RefreshTokenFactory(); + self::$refreshTokenFactory = $f; + return $f; + } else { + return self::$refreshTokenFactory; + } + } + public static function getRegVoucherFactory(): RegVoucherFactory { if (self::$regVoucherFactory == null) { $f = new RegVoucherFactory(); diff --git a/src/dba/models/RefreshToken.php b/src/dba/models/RefreshToken.php new file mode 100644 index 000000000..351bdda45 --- /dev/null +++ b/src/dba/models/RefreshToken.php @@ -0,0 +1,149 @@ +refreshTokenId = $refreshTokenId; + $this->userId = $userId; + $this->tokenHash = $tokenHash; + $this->familyId = $familyId; + $this->issuedAt = $issuedAt; + $this->endValid = $endValid; + $this->usedAt = $usedAt; + $this->isRevoked = $isRevoked; + } + + function getKeyValueDict(): array { + $dict = array(); + $dict['refreshTokenId'] = $this->refreshTokenId; + $dict['userId'] = $this->userId; + $dict['tokenHash'] = $this->tokenHash; + $dict['familyId'] = $this->familyId; + $dict['issuedAt'] = $this->issuedAt; + $dict['endValid'] = $this->endValid; + $dict['usedAt'] = $this->usedAt; + $dict['isRevoked'] = $this->isRevoked; + + return $dict; + } + + static function getFeatures(): array { + $dict = array(); + $dict['refreshTokenId'] = ['read_only' => True, "type" => "int", "subtype" => "unset", "choices" => "unset", "null" => False, "pk" => True, "protected" => True, "private" => False, "alias" => "refreshTokenId", "public" => False, "dba_mapping" => False]; + $dict['userId'] = ['read_only' => True, "type" => "int", "subtype" => "unset", "choices" => "unset", "null" => False, "pk" => False, "protected" => False, "private" => False, "alias" => "userId", "public" => False, "dba_mapping" => False]; + $dict['tokenHash'] = ['read_only' => True, "type" => "str(64)", "subtype" => "unset", "choices" => "unset", "null" => False, "pk" => False, "protected" => True, "private" => True, "alias" => "tokenHash", "public" => False, "dba_mapping" => False]; + $dict['familyId'] = ['read_only' => True, "type" => "str(32)", "subtype" => "unset", "choices" => "unset", "null" => False, "pk" => False, "protected" => True, "private" => True, "alias" => "familyId", "public" => False, "dba_mapping" => False]; + $dict['issuedAt'] = ['read_only' => True, "type" => "int64", "subtype" => "unset", "choices" => "unset", "null" => False, "pk" => False, "protected" => False, "private" => False, "alias" => "issuedAt", "public" => False, "dba_mapping" => False]; + $dict['endValid'] = ['read_only' => True, "type" => "int64", "subtype" => "unset", "choices" => "unset", "null" => False, "pk" => False, "protected" => False, "private" => False, "alias" => "endValid", "public" => False, "dba_mapping" => False]; + $dict['usedAt'] = ['read_only' => True, "type" => "int64", "subtype" => "unset", "choices" => "unset", "null" => True, "pk" => False, "protected" => False, "private" => False, "alias" => "usedAt", "public" => False, "dba_mapping" => False]; + $dict['isRevoked'] = ['read_only' => True, "type" => "bool", "subtype" => "unset", "choices" => "unset", "null" => False, "pk" => False, "protected" => False, "private" => False, "alias" => "isRevoked", "public" => False, "dba_mapping" => False]; + + return $dict; + } + + function getPrimaryKey(): string { + return "refreshTokenId"; + } + + function getPrimaryKeyValue(): ?int { + return $this->refreshTokenId; + } + + function getId(): ?int { + return $this->refreshTokenId; + } + + function setId($id): void { + $this->refreshTokenId = $id; + } + + /** + * Used to serialize the data contained in the model + * @return array + */ + public function expose(): array { + return get_object_vars($this); + } + + function getUserId(): ?int { + return $this->userId; + } + + function setUserId(?int $userId): void { + $this->userId = $userId; + } + + function getTokenHash(): ?string { + return $this->tokenHash; + } + + function setTokenHash(?string $tokenHash): void { + $this->tokenHash = $tokenHash; + } + + function getFamilyId(): ?string { + return $this->familyId; + } + + function setFamilyId(?string $familyId): void { + $this->familyId = $familyId; + } + + function getIssuedAt(): ?int { + return $this->issuedAt; + } + + function setIssuedAt(?int $issuedAt): void { + $this->issuedAt = $issuedAt; + } + + function getEndValid(): ?int { + return $this->endValid; + } + + function setEndValid(?int $endValid): void { + $this->endValid = $endValid; + } + + function getUsedAt(): ?int { + return $this->usedAt; + } + + function setUsedAt(?int $usedAt): void { + $this->usedAt = $usedAt; + } + + function getIsRevoked(): ?int { + return $this->isRevoked; + } + + function setIsRevoked(?int $isRevoked): void { + $this->isRevoked = $isRevoked; + } + + const REFRESH_TOKEN_ID = "refreshTokenId"; + const USER_ID = "userId"; + const TOKEN_HASH = "tokenHash"; + const FAMILY_ID = "familyId"; + const ISSUED_AT = "issuedAt"; + const END_VALID = "endValid"; + const USED_AT = "usedAt"; + const IS_REVOKED = "isRevoked"; + + const PERM_CREATE = "permRefreshTokenCreate"; + const PERM_READ = "permRefreshTokenRead"; + const PERM_UPDATE = "permRefreshTokenUpdate"; + const PERM_DELETE = "permRefreshTokenDelete"; +} diff --git a/src/dba/models/RefreshTokenFactory.php b/src/dba/models/RefreshTokenFactory.php new file mode 100644 index 000000000..4c3660c88 --- /dev/null +++ b/src/dba/models/RefreshTokenFactory.php @@ -0,0 +1,50 @@ + + */ +class RefreshTokenFactory extends AbstractModelFactory { + function getModelName(): string { + return "RefreshToken"; + } + + function getModelTable(): string { + return "RefreshToken"; + } + + function isMapping(): bool { + return False; + } + + function isCachable(): bool { + return false; + } + + function getCacheValidTime(): int { + return -1; + } + + /** + * @return RefreshToken + */ + function getNullObject(): RefreshToken { + return new RefreshToken(-1, null, null, null, null, null, null, null); + } + + /** + * @param array $dict + * @return RefreshToken + */ + function createObjectFromDict(array $dict): RefreshToken { + $conv = []; + foreach ($dict as $key => $val) { + $conv[strtolower($key)] = $val; + } + $dict = $conv; + return new RefreshToken($dict['refreshtokenid'], $dict['userid'], $dict['tokenhash'], $dict['familyid'], $dict['issuedat'], $dict['endvalid'], $dict['usedat'], $dict['isrevoked']); + } +} diff --git a/src/dba/models/generator.php b/src/dba/models/generator.php index 99ad380f0..0e0e25e2f 100644 --- a/src/dba/models/generator.php +++ b/src/dba/models/generator.php @@ -464,6 +464,18 @@ ['name' => 'crackerBinaryTypeId', 'read_only' => False, 'type' => 'int'], ], ]; +$CONF['RefreshToken'] = [ + 'columns' => [ + ['name' => 'refreshTokenId', 'read_only' => True, 'type' => 'int', 'protected' => True], + ['name' => 'userId', 'read_only' => True, 'type' => 'int', 'relation' => 'User'], + ['name' => 'tokenHash', 'read_only' => True, 'type' => 'str(64)', 'protected' => True, 'private' => True], + ['name' => 'familyId', 'read_only' => True, 'type' => 'str(32)', 'protected' => True, 'private' => True], + ['name' => 'issuedAt', 'read_only' => True, 'type' => 'int64'], + ['name' => 'endValid', 'read_only' => True, 'type' => 'int64'], + ['name' => 'usedAt', 'read_only' => True, 'null' => True, 'type' => 'int64'], + ['name' => 'isRevoked', 'read_only' => True, 'type' => 'bool'], + ], +]; $CONF['RegVoucher'] = [ 'columns' => [ ['name' => 'regVoucherId', 'read_only' => True, 'type' => 'int', 'protected' => True], diff --git a/src/inc/StartupConfig.php b/src/inc/StartupConfig.php index 25b108214..eb1b4f14d 100644 --- a/src/inc/StartupConfig.php +++ b/src/inc/StartupConfig.php @@ -8,6 +8,7 @@ class StartupConfig { private array $directories; private array $db_properties; private array $peppers; + private array $refresh_token; /** * The choice here is to define the possible keys for config settings only private and only allow to @@ -22,6 +23,10 @@ class StartupConfig { private const DIRECTORY_CONFIG = "config"; private const DIRECTORY_TUS = "tus"; + private const REFRESH_TOKEN_LIFETIME = "lifetime"; + private const REFRESH_TOKEN_COOKIE_SECURE = "cookieSecure"; + private const REFRESH_TOKEN_COOKIE_SAMESITE = "cookieSameSite"; + private const DB_PROPERTY_TYPE = "type"; private const DB_PROPERTY_USER = "user"; private const DB_PROPERTY_PASS = "pass"; @@ -68,6 +73,17 @@ public function __construct() { $this->peppers = ["", "", "", ""]; + $this->refresh_token = [ + // 14 days; a session that is not refreshed within this window requires a new login + self::REFRESH_TOKEN_LIFETIME => 14 * 24 * 3600, + // null means the Secure flag follows the scheme the request came in over + self::REFRESH_TOKEN_COOKIE_SECURE => null, + /* Frontend and API normally share a site even when they sit on different ports, and ports do not + make a request cross-site, so Strict holds for the usual deployment. Only a frontend on a + genuinely different domain needs None, which in turn only works on a Secure cookie. */ + self::REFRESH_TOKEN_COOKIE_SAMESITE => "Strict", + ]; + // this is a legacy check for old setups (through manual install) where some settings were stored in the conf.php if (file_exists(dirname(__FILE__) . "/conf.php")) { $this->loadLegacyConfig(); @@ -134,6 +150,21 @@ private function loadEnv(): void { if (getenv('HASHTOPOLIS_TUS_PATH') !== false) { $this->directories[self::DIRECTORY_TUS] = getenv('HASHTOPOLIS_TUS_PATH'); } + + if (getenv('HASHTOPOLIS_REFRESH_TOKEN_LIFETIME') !== false) { + $lifetime = (int)getenv('HASHTOPOLIS_REFRESH_TOKEN_LIFETIME'); + if ($lifetime > 0) { + $this->refresh_token[self::REFRESH_TOKEN_LIFETIME] = $lifetime; + } + } + /* Only needed to overrule the automatic detection, for instance behind a proxy which terminates TLS + without announcing it through X-Forwarded-Proto. */ + if (getenv('HASHTOPOLIS_REFRESH_COOKIE_SECURE') !== false) { + $this->refresh_token[self::REFRESH_TOKEN_COOKIE_SECURE] = filter_var(getenv('HASHTOPOLIS_REFRESH_COOKIE_SECURE'), FILTER_VALIDATE_BOOLEAN); + } + if (in_array(getenv('HASHTOPOLIS_REFRESH_COOKIE_SAMESITE'), ["Strict", "Lax", "None"], true)) { + $this->refresh_token[self::REFRESH_TOKEN_COOKIE_SAMESITE] = getenv('HASHTOPOLIS_REFRESH_COOKIE_SAMESITE'); + } } /** @@ -227,6 +258,24 @@ public function getDatabasePort(): string { return $this->db_properties[self::DB_PROPERTY_PORT]; } + /** + * Lifetime of a refresh token in seconds. Every rotation restarts this window. + */ + public function getRefreshTokenLifetime(): int { + return $this->refresh_token[self::REFRESH_TOKEN_LIFETIME]; + } + + /** + * @return bool|null null when the Secure flag should follow the scheme of the incoming request + */ + public function getRefreshCookieSecure(): ?bool { + return $this->refresh_token[self::REFRESH_TOKEN_COOKIE_SECURE]; + } + + public function getRefreshCookieSameSite(): string { + return $this->refresh_token[self::REFRESH_TOKEN_COOKIE_SAMESITE]; + } + public function getPepper(int $index): string { if ($index < 0 || $index >= count($this->peppers)) { return ""; diff --git a/src/inc/apiv2/auth/JWTBeforeHandler.php b/src/inc/apiv2/auth/JWTBeforeHandler.php index 24bd90a6e..948ff63a0 100644 --- a/src/inc/apiv2/auth/JWTBeforeHandler.php +++ b/src/inc/apiv2/auth/JWTBeforeHandler.php @@ -7,6 +7,7 @@ use Hashtopolis\inc\apiv2\error\HttpError; use Hashtopolis\inc\apiv2\error\HttpForbidden; use Hashtopolis\inc\apiv2\model\ApiTokenAPI; +use Hashtopolis\inc\defines\DTokenType; use JimTools\JwtAuth\Handlers\BeforeHandlerInterface; use Psr\Http\Message\ServerRequestInterface; @@ -18,6 +19,16 @@ class JWTBeforeHandler implements BeforeHandlerInterface { * @throws Exception */ public function __invoke(ServerRequestInterface $request, array $arguments): ServerRequestInterface { + /* Every token this deployment issues is signed with the same key, so a valid signature says only + that we minted it, not what we minted it for. Refuse anything whose declared purpose is not + authorising a resource request, so a credential issued for some other endpoint cannot be spent + here. Tokens issued before the claim existed carry no type and are still accepted; they age + out on their own. */ + $type = $arguments["decoded"]["type"] ?? DTokenType::ACCESS; + if ($type !== DTokenType::ACCESS) { + throw new HttpForbidden("This endpoint needs an access token, but a token of type '$type' was supplied."); + } + if (isset ($arguments["decoded"]["aud"]) && $arguments["decoded"]["aud"] == ApiTokenAPI::API_AUD) { $apiTokenId = $arguments["decoded"]["jti"]; $token = Factory::getJwtApiKeyFactory()->get($apiTokenId); diff --git a/src/inc/apiv2/auth/RefreshTokenCookie.php b/src/inc/apiv2/auth/RefreshTokenCookie.php new file mode 100644 index 000000000..2c5fc7d53 --- /dev/null +++ b/src/inc/apiv2/auth/RefreshTokenCookie.php @@ -0,0 +1,110 @@ +getCookieParams(); + if (!isset($cookies[self::COOKIE_NAME]) || !is_string($cookies[self::COOKIE_NAME])) { + return null; + } + $value = trim($cookies[self::COOKIE_NAME]); + + return $value === "" ? null : $value; + } + + /** + * Attaches a refresh token to the response. + * + * @param Request $request the request being answered, used to decide on the Secure flag + * @param Response $response + * @param string $token the token string to hand to the client + * @return Response + */ + public static function attach(Request $request, Response $response, string $token): Response { + $maxAge = StartupConfig::getInstance()->getRefreshTokenLifetime(); + + return $response->withAddedHeader("Set-Cookie", self::build($request, $token, $maxAge)); + } + + /** + * Instructs the client to drop the refresh token. The attributes have to match those used when the + * cookie was set, otherwise the browser keeps the original cookie around. + * + * @param Request $request the request being answered, used to decide on the Secure flag + * @param Response $response + * @return Response + */ + public static function clear(Request $request, Response $response): Response { + return $response->withAddedHeader("Set-Cookie", self::build($request, "", 0)); + } + + /** + * @param Request $request + * @param string $value + * @param int $maxAge lifetime in seconds; 0 expires the cookie immediately + * @return string a Set-Cookie header value + */ + private static function build(Request $request, string $value, int $maxAge): string { + $config = StartupConfig::getInstance(); + $sameSite = $config->getRefreshCookieSameSite(); + + $parts = [ + self::COOKIE_NAME . "=" . $value, + "Path=" . self::COOKIE_PATH, + "Max-Age=" . $maxAge, + "Expires=" . gmdate("D, d M Y H:i:s \G\M\T", time() + $maxAge), + "HttpOnly", + "SameSite=" . $sameSite, + ]; + /* SameSite=None is only honoured on cookies which are also Secure, so the two cannot be + configured against each other without the browser silently dropping the cookie. */ + if (self::isSecure($request) || $sameSite === "None") { + $parts[] = "Secure"; + } + + return implode("; ", $parts); + } + + /** + * Whether the cookie should be marked Secure. Flagging it on a plain HTTP deployment would make the + * client withhold the cookie from every subsequent request, so this follows the scheme the request + * actually arrived over unless the deployment says otherwise. + * + * @param Request $request + * @return bool + */ + private static function isSecure(Request $request): bool { + $configured = StartupConfig::getInstance()->getRefreshCookieSecure(); + if ($configured !== null) { + return $configured; + } + + if ($request->getUri()->getScheme() === "https") { + return true; + } + /* A TLS terminating proxy forwards plain HTTP, and only this header says what the client used. */ + $forwarded = explode(",", $request->getHeaderLine("X-Forwarded-Proto"))[0]; + + return strtolower(trim($forwarded)) === "https"; + } +} diff --git a/src/inc/apiv2/auth/token.routes.php b/src/inc/apiv2/auth/token.routes.php index f4c7f70af..f20a081e9 100644 --- a/src/inc/apiv2/auth/token.routes.php +++ b/src/inc/apiv2/auth/token.routes.php @@ -2,8 +2,12 @@ use Firebase\JWT\JWT; +use Hashtopolis\inc\apiv2\auth\RefreshTokenCookie; use Hashtopolis\inc\apiv2\error\HttpError; +use Hashtopolis\inc\defines\DTokenType; +use Hashtopolis\inc\apiv2\error\HttpUnauthorized; use Hashtopolis\inc\StartupConfig; +use Hashtopolis\inc\utils\RefreshTokenUtils; use Psr\Http\Message\ResponseInterface as Response; use Psr\Http\Message\ServerRequestInterface as Request; @@ -15,75 +19,105 @@ use Hashtopolis\dba\models\User; use Hashtopolis\dba\Factory; use Firebase\JWT\JWK; -use Hashtopolis\dba\JoinFilter; -use Hashtopolis\dba\models\RightGroup; use Hashtopolis\inc\apiv2\error\HttpForbidden; +use Hashtopolis\inc\apiv2\util\CorsHackMiddleware; require_once(dirname(__FILE__) . "/../../startup/include.php"); const USER_AUD = "user_hashtopolis"; /** - * @throws HttpError + * Lifetime of an access token in seconds. Access tokens are not revocable, so this is the window in + * which a leaked one stays usable; clients are expected to keep it short and lean on the refresh + * token at /api/v2/auth/refresh to stay logged in beyond it. + */ +const ACCESS_TOKEN_LIFETIME = 2 * 3600; + +/** + * Mints an access token for a user which has already been authenticated. + * + * @param User $user + * @param int $expires unix timestamp at which the token stops being accepted + * @return string the encoded JWT + * @throws HttpForbidden when the user has been deactivated + * @throws HttpError when the user has no right group * @throws RandomException - * @throws HttpForbidden * @throws Exception */ -function generateTokenForUser(Request $request, string $userName, int $expires): string { - $jti = bin2hex(random_bytes(16)); - - $filter = new QueryFilter(User::USERNAME, $userName, "="); - $jF = new JoinFilter(Factory::getRightGroupFactory(), User::RIGHT_GROUP_ID, RightGroup::RIGHT_GROUP_ID); - $joined = Factory::getUserFactory()->filter([Factory::FILTER => $filter, Factory::JOIN => $jF]); - /** @var User[] $check */ - $check = $joined[Factory::getUserFactory()->getModelName()]; - if (count($check) === 0) { - throw new HttpError("No user with this userName in the database"); - } - $user = $check[0]; +function generateAccessToken(User $user, int $expires): string { if ($user->getIsValid() !== 1) { throw new HttpForbidden("User is set to invalid"); } - - /** @var RightGroup[] $groupArray */ - $groupArray = $joined[Factory::getRightGroupFactory()->getModelName()]; - if (count($groupArray) === 0) { + + $group = Factory::getRightGroupFactory()->get($user->getRightGroupId()); + if ($group === null) { throw new HttpError("No rightgroup found for this user"); } - $group = $groupArray[0]; - $scopes = $group->getPermissions(); - - // $requested_scopes = $request->getParsedBody() ?: ["todo.all"]; - // $valid_scopes = [ - // "todo.create", - // "todo.read", - // "todo.update", - // "todo.delete", - // "todo.list", - // "todo.all" - // ]; - // $scopes = array_filter($requested_scopes, function ($needle) use ($valid_scopes) { - // return in_array($needle, $valid_scopes); - // }); - $secret = StartupConfig::getInstance()->getPepper(0); - $now = new DateTime(); - $payload = [ - "iat" => $now->getTimeStamp(), + "iat" => time(), "exp" => $expires, - "jti" => $jti, + "jti" => bin2hex(random_bytes(16)), "userId" => $user->getId(), - "scope" => $scopes, + "scope" => $group->getPermissions(), "iss" => "Hashtopolis", - "kid" => hash("sha256", $secret), - "aud" => USER_AUD + "kid" => hash("sha256", $secret), + "aud" => USER_AUD, + // Says what this token authorises, so it cannot be presented where a different type is expected + "type" => DTokenType::ACCESS ]; return JWT::encode($payload, $secret, "HS256"); } +/** + * @param string $userName + * @return User + * @throws HttpError when no such user exists + * @throws Exception + */ +function findUserByName(string $userName): User { + $filter = new QueryFilter(User::USERNAME, $userName, "="); + $user = Factory::getUserFactory()->filter([Factory::FILTER => $filter], true); + if ($user === null) { + throw new HttpError("No user with this userName in the database"); + } + + return $user; +} + +/** + * Mints an access token for a user identified by name. + * + * @param string $userName + * @param int $expires unix timestamp at which the token stops being accepted + * @return string the encoded JWT + * @throws HttpError when no such user exists + * @throws HttpForbidden + * @throws RandomException + * @throws Exception + */ +function generateTokenForUser(string $userName, int $expires): string { + return generateAccessToken(findUserByName($userName), $expires); +} + +/** + * Builds the response body shared by every endpoint handing out an access token. The refresh token + * itself is deliberately absent: it only ever travels in an HttpOnly cookie. + * + * @param Response $response + * @param string $token the encoded JWT + * @param int $expires unix timestamp at which the token stops being accepted + * @return Response + */ +function accessTokenResponse(Response $response, string $token, int $expires): Response { + $response->getBody()->write(json_encode(["token" => $token, "expires" => $expires], JSON_UNESCAPED_SLASHES)); + + return $response->withStatus(201) + ->withHeader("Content-Type", "application/json"); +} + function extractBearerToken(Request $request): ?string { $header = $request->getHeaderLine('Authorization'); @@ -124,16 +158,11 @@ function extractBearerToken(Request $request): ?string { } $userName = $decoded_jwt->preferred_username; - $future = new DateTime("now +2 hours"); - $token = generateTokenForUser($request, $userName, $future->getTimestamp()); - $data["token"] = $token; - $data["expires"] = $future->getTimestamp(); - - $body = $response->getBody(); - $body->write(json_encode($data, JSON_UNESCAPED_SLASHES)); + $user = findUserByName($userName); + $expires = time() + ACCESS_TOKEN_LIFETIME; + $response = accessTokenResponse($response, generateAccessToken($user, $expires), $expires); - return $response->withStatus(201) - ->withHeader("Content-Type", "application/json"); + return RefreshTokenCookie::attach($request, $response, RefreshTokenUtils::issue($user->getId())); }); }); @@ -145,21 +174,22 @@ function extractBearerToken(Request $request): ?string { }); $group->post('', function (Request $request, Response $response, array $args): Response { + $userName = $request->getAttribute('user'); - $future = new DateTime("now +2 hours"); - $token = generateTokenForUser($request, $request->getAttribute('user'), $future->getTimestamp()); - - $data["token"] = $token; - $data["expires"] = $future->getTimestamp(); + $user = findUserByName($userName); + $expires = time() + ACCESS_TOKEN_LIFETIME; + $response = accessTokenResponse($response, generateAccessToken($user, $expires), $expires); - $body = $response->getBody(); - $body->write(json_encode($data, JSON_UNESCAPED_SLASHES)); - - return $response->withStatus(201) - ->withHeader("Content-Type", "application/json"); + /* Starts a new refresh token family, so logging in again leaves sessions on other devices alone. */ + return RefreshTokenCookie::attach($request, $response, RefreshTokenUtils::issue($user->getId())); }); }); +/* + * Exchanges the refresh token cookie for a fresh access token. This endpoint is exempt from the JWT + * middleware on purpose: its whole reason to exist is to work once the access token has expired, so + * the cookie is the only credential it looks at. + */ $app->group("/api/v2/auth/refresh", function (RouteCollectorProxy $group) { /* Allow preflight requests */ $group->options('', function (Request $request, Response $response, array $args): Response { @@ -167,32 +197,35 @@ function extractBearerToken(Request $request): ?string { }); $group->post('', function (Request $request, Response $response, array $args): Response { - $now = new DateTime(); - $future = new DateTime("now +2 hours"); - - $jti = bin2hex(random_bytes(16)); - - $secret = StartupConfig::getInstance()->getPepper(0); - $payload = [ - "iat" => $now->getTimeStamp(), - "exp" => $future->getTimeStamp(), - "jti" => $jti, - "userId" => $request->getAttribute(('userId')), - "scope" => $request->getAttribute("scope"), - "iss" => "Hashtopolis", - "kid" => hash("sha256", $secret), - "aud" => USER_AUD - ]; + /* This handler acts on the cookie alone, so a page on another origin could otherwise have a + visitor's browser spend their refresh token. The attacker never gets to read the reply, but + spending the token is enough: the victim's next renewal looks like a replay and ends every + session of that login. */ + CorsHackMiddleware::assertNotCrossSite($request); + + $presented = RefreshTokenCookie::read($request); + if ($presented === null) { + throw new HttpUnauthorized("No refresh token supplied"); + } - $token = JWT::encode($payload, $secret, "HS256"); + $rotated = RefreshTokenUtils::rotate($presented); - $data["token"] = $token; - $data["expires"] = $future->getTimeStamp(); + $expires = time() + ACCESS_TOKEN_LIFETIME; + $response = accessTokenResponse($response, generateAccessToken($rotated["user"], $expires), $expires); - $body = $response->getBody(); - $body->write(json_encode($data, JSON_UNESCAPED_SLASHES)); + return RefreshTokenCookie::attach($request, $response, $rotated["token"]); + }); + + /* Logout: ends the session the cookie belongs to and drops the cookie. */ + $group->delete('', function (Request $request, Response $response, array $args): Response { + // Likewise: ending somebody's session on their behalf is a forced logout + CorsHackMiddleware::assertNotCrossSite($request); + + $presented = RefreshTokenCookie::read($request); + if ($presented !== null) { + RefreshTokenUtils::revoke($presented); + } - return $response->withStatus(201) - ->withHeader("Content-Type", "application/json"); + return RefreshTokenCookie::clear($request, $response)->withStatus(204); }); }); diff --git a/src/inc/apiv2/error/HttpUnauthorized.php b/src/inc/apiv2/error/HttpUnauthorized.php new file mode 100644 index 000000000..467f450c4 --- /dev/null +++ b/src/inc/apiv2/error/HttpUnauthorized.php @@ -0,0 +1,15 @@ + json_encode($requestedScopes), "iss" => "Hashtopolis", "aud" => $this::API_AUD, - "kid" => hash("sha256", $secret) + "kid" => hash("sha256", $secret), + // An API token authorises resource requests, the same as a login token does + "type" => DTokenType::ACCESS ]; $tokenEncoded = JWT::encode($payload, $secret, "HS256"); diff --git a/src/inc/apiv2/openapi/SpecBuilder.php b/src/inc/apiv2/openapi/SpecBuilder.php index 7073ffabe..a0c9a7791 100644 --- a/src/inc/apiv2/openapi/SpecBuilder.php +++ b/src/inc/apiv2/openapi/SpecBuilder.php @@ -67,6 +67,7 @@ public function buildFromApp(App $app): array { * Build static entries */ $paths["/api/v2/auth/token"] = $this->staticFragments->authTokenPath(); + $paths["/api/v2/auth/refresh"] = $this->staticFragments->authRefreshPath(); foreach ($this->staticFragments->tokenComponents() as $key => $schema) { $components[$key] = $schema; @@ -115,6 +116,12 @@ public function buildFromApp(App $app): array { "type" => "http", "description" => "Basic Authorization header.", "scheme" => "basic" + ], + "refreshCookie" => [ + "type" => "apiKey", + "description" => "HttpOnly cookie holding the refresh token, set by /api/v2/auth/token and scoped to /api/v2/auth/refresh.", + "in" => "cookie", + "name" => "refreshToken" ] ] ], diff --git a/src/inc/apiv2/openapi/SpecSanitizer.php b/src/inc/apiv2/openapi/SpecSanitizer.php index 602290aeb..ac2fff735 100644 --- a/src/inc/apiv2/openapi/SpecSanitizer.php +++ b/src/inc/apiv2/openapi/SpecSanitizer.php @@ -49,6 +49,11 @@ public function sanitize(array $spec): array { // Fix: Security requirement - bearerAuth should have empty scopes array for HTTP bearer if (isset($operation['security'])) { foreach ($operation['security'] as &$secReq) { + // An empty requirement is the OpenAPI spelling of "this operation may be called + // unauthenticated", and is an object rather than a map of schemes to scopes + if (!is_array($secReq)) { + continue; + } if (isset($secReq['bearerAuth'])) { $secReq['bearerAuth'] = []; } diff --git a/src/inc/apiv2/openapi/StaticFragments.php b/src/inc/apiv2/openapi/StaticFragments.php index e65602d8b..c1b4e8b5f 100644 --- a/src/inc/apiv2/openapi/StaticFragments.php +++ b/src/inc/apiv2/openapi/StaticFragments.php @@ -80,6 +80,104 @@ public function authTokenPath(): array { ]; } + /** + * The refresh endpoint trades the refresh token cookie for a new access token + * without asking for credentials again, and its DELETE ends the session the + * cookie belongs to (see token.routes.php). + */ + public function authRefreshPath(): array { + return [ + "post" => [ + "tags" => [ + "Login" + ], + "summary" => "Exchange the refresh token cookie for a new access token", + "description" => "Reads the refreshToken cookie set by /api/v2/auth/token, rotates it and answers + with a new access token. Needs no Authorization header, so it keeps working once the previous + access token has expired. The rotated cookie is returned in Set-Cookie; the token itself is + never part of the body.", + "responses" => [ + "201" => [ + "description" => "Success", + "headers" => [ + "Set-Cookie" => $this->rotatedCookieHeader() + ], + "content" => [ + "application/json" => [ + "schema" => [ + '$ref' => "#/components/schemas/Token" + ] + ] + ] + ], + "401" => $this->problemResponse("The refresh token is missing, expired, revoked or already used"), + "403" => $this->problemResponse("The user has been deactivated") + ], + "security" => [ + [ + "refreshCookie" => [] + ] + ] + ], + "delete" => [ + "tags" => [ + "Login" + ], + "summary" => "Log out", + "description" => "Revokes the session the refreshToken cookie belongs to and clears the cookie. + Sessions on other devices are left alone. Logging out without a cookie is not an error.", + "responses" => [ + "204" => [ + "description" => "Success", + "headers" => [ + "Set-Cookie" => $this->clearedCookieHeader() + ] + ], + "403" => $this->problemResponse("The request origin is not allowed to send credentials") + ], + /* Logging out without a cookie is a successful no-op, so the cookie cannot be a hard + requirement here: the empty alternative is how OpenAPI spells "optional", and without it a + generated client would refuse to make a call the server answers with 204. */ + "security" => [ + [ + "refreshCookie" => [] + ], + new \stdClass() + ] + ] + ]; + } + + /** + * The cookie handed out on a successful exchange. Its lifetime follows the deployment's configured + * refresh token lifetime, so the Max-Age in the example is the default rather than a fixed value. + */ + private function rotatedCookieHeader(): array { + return [ + "description" => "The rotated refresh token, scoped to /api/v2/auth/refresh and marked HttpOnly. + Replaces the cookie sent with the request, which is consumed by this call.", + "schema" => [ + "type" => "string", + "example" => "refreshToken=4fa1371293a112224bc930cf9fecd0fd; Path=/api/v2/auth/refresh; Max-Age=1209600; Expires=Wed, 30 Sep 2026 07:02:31 GMT; HttpOnly; SameSite=Strict" + ] + ]; + } + + /** + * Logging out sends the same cookie back empty and already expired, which is how a client is told + * to drop it. Describing it as the rotated cookie would document the opposite of what happens. + */ + private function clearedCookieHeader(): array { + return [ + "description" => "The refresh token cookie, emptied and expired so the client drops it. Carries + the same attributes it was set with, which is what makes a browser replace rather than keep it.", + "schema" => [ + "type" => "string", + "example" => "refreshToken=; Path=/api/v2/auth/refresh; Max-Age=0; Expires=Wed, 16 Sep 2026 07:02:31 GMT; HttpOnly; SameSite=Strict" + ] + ]; + } + /** * Errors are rendered as RFC 7807 problem documents by * ErrorHandler::errorResponse, on every APIv2 route. diff --git a/src/inc/apiv2/util/CorsHackMiddleware.php b/src/inc/apiv2/util/CorsHackMiddleware.php index 8448aafa3..2d6aa688b 100644 --- a/src/inc/apiv2/util/CorsHackMiddleware.php +++ b/src/inc/apiv2/util/CorsHackMiddleware.php @@ -17,6 +17,12 @@ class CorsHackMiddleware implements MiddlewareInterface { * @throws HttpForbidden */ public function process(Request $request, RequestHandler $handler): Response { + /* Decide about the origin before handing the request on. This check used to run on the way back + out, which meant a forged cross-site request reached its handler and took effect - a refresh + token spent, a session ended - and only then collected its 403. Refusing after the fact still + denies the attacker the reply, but by then the damage is done. */ + self::resolveAllowedOrigin($request); + $response = $handler->handle($request); return CorsHackMiddleware::addCORSHeaders($request, $response); @@ -34,60 +40,249 @@ public static function addCORSHeaders(Request $request, $response) { $response = CorsHackMiddleware::CheckCORS($request, $response); - // Optional: Allow Ajax CORS requests with Authorization header - // $response = $response->withHeader('Access-Control-Allow-Credentials', 'true'); - $response = $response->withHeader('Access-Control-Allow-Methods', implode(',', $methods)); return $response->withHeader('Access-Control-Allow-Headers', $requestHeaders); } /** - * @throws HttpForbidden + * Decides which origin, if any, the response may be shared with. + * + * An origin is accepted only when its scheme, host and effective port all match one of the origins + * this deployment is configured to trust. Comparing the parts separately matters: the previous + * string slicing reduced any portless URL to an empty host, so an arbitrary origin compared equal + * to a portless HASHTOPOLIS_BACKEND_URL and was echoed back as trusted. + * + * Matching is deliberately exact. There is no wildcard and no suffix matching, because a trusted + * origin is handed credentials, and `https://app.example.com` must not admit + * `https://app.example.com.evil.test`. + * + * @throws HttpForbidden when an origin is supplied that the deployment does not recognise */ public static function CheckCORS($request, $response): Response { - $requestHttpOrigin = $request->getHeaderLine('HTTP_ORIGIN'); - - $envBackend = getenv('HASHTOPOLIS_BACKEND_URL'); - $envFrontendPort = getenv('HASHTOPOLIS_FRONTEND_PORT'); - - if (($envBackend !== false || $envFrontendPort !== false) && $requestHttpOrigin != "") { - $requestHttpOrigin = explode('://', $requestHttpOrigin)[1]; - - $envBackend = explode('://', $envBackend)[1]; - $envBackend = explode('/', $envBackend)[0]; - - $requestHttpOriginUrl = substr($requestHttpOrigin, 0, strrpos($requestHttpOrigin, ":")); //Needs to use strrpos in case of ipv6 because of multiple ':' characters - $envBackendUrl = substr($envBackend, 0, strrpos($envBackend, ":")); - - $localhostSynonyms = ["localhost", "127.0.0.1", "[::1]"]; - - if ($requestHttpOriginUrl === $envBackendUrl || (in_array($requestHttpOriginUrl, $localhostSynonyms) && in_array($envBackendUrl, $localhostSynonyms))) { - //Origin URL matches, now check the port too - if (!str_ends_with($requestHttpOrigin, "]") && str_contains($requestHttpOrigin, ":")) { - $requestHttpOriginPort = substr($requestHttpOrigin, strrpos($requestHttpOrigin, ":") + 1); //Needs to use strrpos in case of ipv6 because of multiple ':' characters - $envBackendPort = substr($envBackend, strrpos($envBackend, ":") + 1); - - if ($requestHttpOriginPort === $envFrontendPort || $requestHttpOriginPort === $envBackendPort) { - $response = $response->withHeader('Access-Control-Allow-Origin', $request->getHeaderLine('HTTP_ORIGIN')); - } - else { - throw new HttpForbidden("CORS error: Allow-Origin port doesn't match: the value from the request is $requestHttpOriginPort but expected $envFrontendPort or $envBackendPort. Try switching the frontend port back to the default value (4200) in the docker-compose."); - } + $allowedOrigin = self::resolveAllowedOrigin($request); + + return $allowedOrigin === null + ? $response->withHeader('Access-Control-Allow-Origin', '*') + : self::allowOrigin($allowedOrigin, $response); + } + + /** + * Makes the same decision without needing the response in hand, so it can be made before a handler + * runs as well as after. + * + * @return string|null the origin to name in the response, or null when the answer is the wildcard + * @throws HttpForbidden when an origin is supplied that the deployment does not recognise + */ + private static function resolveAllowedOrigin($request): ?string { + $requestHttpOrigin = $request->getHeaderLine('Origin'); + + $allowed = self::allowedOrigins(); + + /* With nothing configured there is nothing to check an origin against, so the API stays readable + from anywhere but never on a credentialed request: browsers reject credentials next to a + wildcard, which is also why the refresh token cookie needs one of the settings below. */ + if (count($allowed) === 0 || $requestHttpOrigin === "") { + return null; + } + + $origin = self::parseOrigin($requestHttpOrigin); + if ($origin === null) { + throw new HttpForbidden("CORS error: the request Origin '$requestHttpOrigin' is not a usable http(s) origin."); + } + + foreach ($allowed as $candidate) { + if (self::originsMatch($origin, $candidate)) { + return $requestHttpOrigin; + } + } + + $expected = implode(', ', array_map(self::describeOrigin(...), $allowed)); + throw new HttpForbidden("CORS error: the request Origin '$requestHttpOrigin' does not match this deployment. Allowed origins are: $expected. Check HASHTOPOLIS_BACKEND_URL, HASHTOPOLIS_FRONTEND_URLS and HASHTOPOLIS_FRONTEND_PORT."); + } + + /** + * Refuses a request that a browser made from an origin other than this deployment's. + * + * Endpoints authenticating from a cookie need this and endpoints authenticating from a bearer token + * do not: a cookie rides along on whatever request a page chooses to make, so a page elsewhere can + * have a visitor's browser act as them. The check above already refuses unknown origins, but only + * once an allow-list exists; with none configured it answers the wildcard, which is right for a + * token API and wrong for a cookie one. + * + * @throws HttpForbidden when the request came from another origin + */ + public static function assertNotCrossSite($request): void { + $requestHttpOrigin = trim($request->getHeaderLine('Origin')); + + // A client sending no origin is not a browser, so it carries no ambient cookie to be abused + if ($requestHttpOrigin === "") { + return; + } + + if (count(self::allowedOrigins()) > 0) { + self::resolveAllowedOrigin($request); + return; + } + + /* Nothing is configured to compare against, so fall back to the host the request was addressed + to. A page on a neighbouring host is a different origin and is refused here, which is exactly + the case SameSite=Strict on the cookie does not cover. */ + $origin = self::parseOrigin($requestHttpOrigin); + if ($origin !== null && self::isSameHost($origin['host'], strtolower($request->getUri()->getHost()))) { + return; + } + + throw new HttpForbidden("This endpoint cannot be called from another origin. The request came from '$requestHttpOrigin'."); + } + + /** + * Resolves every origin this deployment trusts, from the three settings that can name one. + * + * HASHTOPOLIS_FRONTEND_PORT is folded in as a derived origin rather than handled as a special case + * further down, so there is a single matching rule and the legacy setting cannot drift from the + * list. It names a port on the API's own host, so it only contributes when the backend URL is + * known. + * + * @return list + * @throws HttpForbidden when a setting names something that is not an http(s) origin + */ + private static function allowedOrigins(): array { + $envBackend = self::readSetting('HASHTOPOLIS_BACKEND_URL'); + $envFrontendUrls = self::readSetting('HASHTOPOLIS_FRONTEND_URLS'); + $envFrontendPort = self::readSetting('HASHTOPOLIS_FRONTEND_PORT'); + + $allowed = []; + $backend = null; + + if ($envBackend !== null) { + $backend = self::parseOrigin($envBackend); + if ($backend === null) { + throw new HttpForbidden("CORS error: HASHTOPOLIS_BACKEND_URL ('$envBackend') is not a usable http(s) URL. It should look like 'https://hashtopolis.example.com' or 'http://localhost:8080'."); + } + $allowed[] = $backend; + } + + if ($envFrontendUrls !== null) { + foreach (explode(',', $envFrontendUrls) as $entry) { + $entry = trim($entry); + // A trailing comma carries no intent and names nothing that could be reported + if ($entry === "") { + continue; } - else { - //No port given in the request origin, all checks passed - $response = $response->withHeader('Access-Control-Allow-Origin', $request->getHeaderLine('HTTP_ORIGIN')); + + $parsed = self::parseOrigin($entry); + if ($parsed === null) { + throw new HttpForbidden("CORS error: HASHTOPOLIS_FRONTEND_URLS contains '$entry', which is not a usable http(s) origin. Each entry should look like 'https://app.example.com' or 'http://localhost:4200'."); } + $allowed[] = $parsed; + } + } + + if ($envFrontendPort !== null && ctype_digit($envFrontendPort)) { + if ($backend === null) { + /* Without a backend URL there is no host to attach the port to. The shipped compose files + hardcode this setting, so refusing the request would break every deployment that adopts + HASHTOPOLIS_FRONTEND_URLS; say so in the log and carry on with the origins we do have. */ + error_log("HASHTOPOLIS_FRONTEND_PORT is set but HASHTOPOLIS_BACKEND_URL is not, so there is no host to apply the port to. Name the frontend in HASHTOPOLIS_FRONTEND_URLS instead."); } else { - throw new HttpForbidden("CORS error: Allow-Origin URL doesn't match: the value from the request is $requestHttpOriginUrl but expected $envBackendUrl. Is the HASHTOPOLIS_BACKEND_URL in the .env file the correct one?"); + $allowed[] = ['scheme' => $backend['scheme'], 'host' => $backend['host'], 'port' => (int)$envFrontendPort]; } } - else { - //No backend URL given in .env file or no origin supplied in the request, switch to default allow all - $response = $response->withHeader('Access-Control-Allow-Origin', '*'); + + return $allowed; + } + + /** + * Reads a deployment setting, treating a variable that is present but empty as one that was never + * set. + * + * Docker Compose writes an empty value for every `FOO: $FOO` entry whose variable is missing from + * the .env file, and getenv() answers "" for that rather than false. Without this, blanking a + * setting turns into a configuration error reported on every request, naming a variable the + * operator deliberately left empty. + * + * @param string $name + * @return string|null the trimmed value, or null when unset, empty or only whitespace + */ + private static function readSetting(string $name): ?string { + $value = getenv($name); + if ($value === false) { + return null; } + $value = trim($value); - return $response; + return $value === "" ? null : $value; + } + + /** + * Splits a URL into the three parts that make up an origin, filling in the port the scheme implies + * when the URL leaves it out, so that https://example.com and https://example.com:443 compare equal. + * + * @return array{scheme: string, host: string, port: int}|null null when this is not an http(s) URL + */ + private static function parseOrigin(string $url): ?array { + $parts = parse_url(trim($url)); + if ($parts === false || !isset($parts['scheme'], $parts['host'])) { + return null; + } + + $scheme = strtolower($parts['scheme']); + $defaultPorts = ['http' => 80, 'https' => 443]; + if (!array_key_exists($scheme, $defaultPorts)) { + return null; + } + + return [ + 'scheme' => $scheme, + 'host' => strtolower($parts['host']), + 'port' => isset($parts['port']) ? (int)$parts['port'] : $defaultPorts[$scheme] + ]; + } + + /** + * @param array{scheme: string, host: string, port: int} $origin the origin the request came from + * @param array{scheme: string, host: string, port: int} $candidate an origin this deployment trusts + */ + private static function originsMatch(array $origin, array $candidate): bool { + return $origin['scheme'] === $candidate['scheme'] + && $origin['port'] === $candidate['port'] + && self::isSameHost($origin['host'], $candidate['host']); + } + + /** + * @param array{scheme: string, host: string, port: int} $origin + */ + private static function describeOrigin(array $origin): string { + return $origin['scheme'] . '://' . $origin['host'] . ':' . $origin['port']; + } + + /** + * The loopback spellings all name the same machine, and a development setup routinely mixes them. + */ + private static function isSameHost(string $origin, string $backend): bool { + if ($origin === $backend) { + return true; + } + + $localhostSynonyms = ["localhost", "127.0.0.1", "[::1]"]; + + return in_array($origin, $localhostSynonyms, true) && in_array($backend, $localhostSynonyms, true); + } + + /** + * Echoes back a single, verified origin and allows the browser to send credentials along with it. + * Without Allow-Credentials the refresh token cookie would never reach /api/v2/auth/refresh from a + * frontend served on another origin, and the header is only accepted next to a concrete origin. + * + * Vary tells caches that this response is specific to the origin that asked for it, so one origin's + * response is never handed to another. + * + * @param string $origin an origin that has already been checked against this deployment + */ + private static function allowOrigin(string $origin, Response $response): Response { + return $response->withHeader('Access-Control-Allow-Origin', $origin) + ->withHeader('Access-Control-Allow-Credentials', 'true') + ->withAddedHeader('Vary', 'Origin'); } -} \ No newline at end of file +} diff --git a/src/inc/defines/DTokenType.php b/src/inc/defines/DTokenType.php new file mode 100644 index 000000000..555451e2d --- /dev/null +++ b/src/inc/defines/DTokenType.php @@ -0,0 +1,19 @@ +mset($user, [User::PASSWORD_HASH => $newHash, User::PASSWORD_SALT => $newSalt, USer::IS_COMPUTED_PASSWORD => 0]); + RefreshTokenUtils::revokeAllForUser($user->getId()); Util::createLogEntry(DLogEntryIssuer::USER, $user->getId(), DLogEntry::INFO, "User changed password!"); } diff --git a/src/inc/utils/RefreshTokenUtils.php b/src/inc/utils/RefreshTokenUtils.php new file mode 100644 index 000000000..3154efa46 --- /dev/null +++ b/src/inc/utils/RefreshTokenUtils.php @@ -0,0 +1,256 @@ +getRefreshTokenLifetime(), + null, + 0 + ); + Factory::getRefreshTokenFactory()->save($token); + + return $plain; + } + + /** + * Exchanges a refresh token for its successor. + * + * The presented token is consumed, so it cannot be exchanged a second time. The caller is expected + * to mint a new access token for the returned user and hand the returned token string back to the + * client. + * + * @param string $plain the token string as presented by the client + * @return array{user: User, token: string} the owning user and the replacement token string + * @throws HttpUnauthorized when the token is unknown, expired, revoked or replayed + * @throws HttpForbidden when the owning user has been deactivated + * @throws Exception + */ + public static function rotate(string $plain): array { + $token = self::findByPlain($plain); + if ($token === null) { + throw new HttpUnauthorized("Refresh token is not valid"); + } + + $now = time(); + if ($token->getEndValid() < $now) { + throw new HttpUnauthorized("Refresh token has expired"); + } + + if (!self::claim($token, $now)) { + // Somebody else consumed the token first, so this is the second use of a single-use token + self::resolveLostClaim($plain); + } + + $user = Factory::getUserFactory()->get($token->getUserId()); + if ($user === null) { + self::revokeFamily($token->getFamilyId()); + throw new HttpUnauthorized("Refresh token is not valid"); + } + if ($user->getIsValid() != 1) { + self::revokeFamily($token->getFamilyId()); + throw new HttpForbidden("Cannot log in. Please contact your administrator for further information"); + } + + return [ + "user" => $user, + "token" => self::issue($token->getUserId(), $token->getFamilyId()), + ]; + } + + /** + * Marks a token as consumed, but only while it is still unused and unrevoked. + * + * The condition lives in the UPDATE rather than in a preceding read, so the database elects exactly + * one winner when requests arrive together. Reading usedAt first would let two requests both see + * null and both go on to issue a successor, which is precisely the case replay detection exists to + * catch: a stolen token racing a legitimate request would never be seen as a second use. + * + * @param RefreshToken $token + * @param int $now + * @return bool whether this caller is the one that consumed the token + * @throws Exception + */ + private static function claim(RefreshToken $token, int $now): bool { + return Factory::getRefreshTokenFactory()->compareAndSet( + $token, + [RefreshToken::USED_AT => null, RefreshToken::IS_REVOKED => 0], + [RefreshToken::USED_AT => $now] + ); + } + + /** + * Decides what losing the claim means, from the state the winner left behind. + * + * Returning means the loss was a client firing two refreshes at once and the caller may carry on; + * Losing it always ends the session: either the token was revoked, or it has already been + * exchanged, and a token exchanged twice is a token two parties hold. + * + * @param string $plain the token string as presented by the client + * @throws HttpUnauthorized always; which error it is depends on what the winner left behind + * @throws Exception + */ + private static function resolveLostClaim(string $plain): void { + $current = self::findByPlain($plain); + if ($current === null || $current->getIsRevoked() == 1) { + throw new HttpUnauthorized("Refresh token has been revoked"); + } + + /* The token was already exchanged, so somebody else is holding a copy of it. There is no way to + tell the legitimate holder from the attacker, so the whole session goes. */ + self::revokeFamily($current->getFamilyId()); + Util::createLogEntry( + DLogEntryIssuer::USER, + (string)$current->getUserId(), + DLogEntry::WARN, + "Refresh token replay detected, all sessions of this login have been revoked!" + ); + throw new HttpUnauthorized("Refresh token has already been used"); + } + + /** + * Ends the session a token belongs to. Used on logout; unknown tokens are ignored so that logging + * out twice is not an error. + * + * @param string $plain the token string as presented by the client + * @throws Exception + */ + public static function revoke(string $plain): void { + $token = self::findByPlain($plain); + if ($token === null) { + return; + } + self::revokeFamily($token->getFamilyId()); + } + + /** + * Revokes every token belonging to one login session, current and past rotations alike. + * + * @param string $familyId + * @throws Exception + */ + public static function revokeFamily(string $familyId): void { + Factory::getRefreshTokenFactory()->massUpdate([ + Factory::FILTER => [new QueryFilter(RefreshToken::FAMILY_ID, $familyId, "=")], + Factory::UPDATE => [new UpdateSet(RefreshToken::IS_REVOKED, 1)] + ]); + } + + /** + * Revokes all sessions of a user, logging them out everywhere. Call this whenever the user's + * credentials or validity change. + * + * @param int $userId + * @throws Exception + */ + public static function revokeAllForUser(int $userId): void { + Factory::getRefreshTokenFactory()->massUpdate([ + Factory::FILTER => [new QueryFilter(RefreshToken::USER_ID, $userId, "=")], + Factory::UPDATE => [new UpdateSet(RefreshToken::IS_REVOKED, 1)] + ]); + } + + /** + * Removes every token of a user. Unlike revoking, this leaves nothing behind, which is what user + * deletion needs since the tokens reference the user row. + * + * @param int $userId + * @throws Exception + */ + public static function deleteAllForUser(int $userId): void { + Factory::getRefreshTokenFactory()->massDeletion([ + Factory::FILTER => [new QueryFilter(RefreshToken::USER_ID, $userId, "=")] + ]); + } + + /** + * Drops tokens which can no longer be exchanged, keeping the table from growing without bound. + * Revoked tokens are kept until they expire so that a replay is still recognised as such. + * + * @param int|null $userId limits the cleanup to one user when given + * @throws Exception + */ + public static function purgeExpired(?int $userId = null): void { + $filters = [new QueryFilter(RefreshToken::END_VALID, time(), "<")]; + if ($userId !== null) { + $filters[] = new QueryFilter(RefreshToken::USER_ID, $userId, "="); + } + Factory::getRefreshTokenFactory()->massDeletion([Factory::FILTER => $filters]); + } + + /** + * @param string $plain the token string as presented by the client + * @return RefreshToken|null + * @throws Exception + */ + private static function findByPlain(string $plain): ?RefreshToken { + if ($plain === "") { + return null; + } + $filter = new QueryFilter(RefreshToken::TOKEN_HASH, self::hashToken($plain), "="); + return Factory::getRefreshTokenFactory()->filter([Factory::FILTER => $filter], true); + } +} diff --git a/src/inc/utils/UserUtils.php b/src/inc/utils/UserUtils.php index 8b377f7d0..4b4c0871a 100644 --- a/src/inc/utils/UserUtils.php +++ b/src/inc/utils/UserUtils.php @@ -65,6 +65,8 @@ public static function deleteUser(int $userId, User $adminUser): void { Factory::getAgentFactory()->massUpdate([Factory::FILTER => $qF, Factory::UPDATE => $uS]); $qF = new QueryFilter(Session::USER_ID, $user->getId(), "="); Factory::getSessionFactory()->massDeletion([Factory::FILTER => $qF]); + // Refresh tokens reference the user row, so they have to go before it does + RefreshTokenUtils::deleteAllForUser($user->getId()); $qF = new QueryFilter(AccessGroupUser::USER_ID, $user->getId(), "="); Factory::getAccessGroupUserFactory()->massDeletion([Factory::FILTER => $qF]); $qF = new QueryFilter(JwtApiKey::USER_ID, $user->getId(), "="); @@ -103,6 +105,7 @@ public static function userForgotPassword(string $username, string $email): void $obj = array('username' => $user->getUsername(), 'password' => $newPass); if (Util::sendMail($user->getEmail(), "Password reset", $tmpl->render($obj), $tmplPlain->render($obj))) { Factory::getUserFactory()->mset($user, [User::PASSWORD_HASH => $newHash, User::PASSWORD_SALT => $newSalt, User::IS_COMPUTED_PASSWORD => 1]); + RefreshTokenUtils::revokeAllForUser($user->getId()); } else { throw new HTException("Password reset failed because of an error when sending the email! Please check if PHP is able to send emails."); @@ -134,6 +137,7 @@ public static function disableUser(int $userId, User $adminUser): void { $qF = new QueryFilter(Session::USER_ID, $user->getId(), "="); $uS = new UpdateSet(Session::IS_OPEN, "0"); Factory::getSessionFactory()->massUpdate([Factory::FILTER => $qF, Factory::UPDATE => $uS]); + RefreshTokenUtils::revokeAllForUser($user->getId()); Factory::getUserFactory()->set($user, User::IS_VALID, 0); } @@ -189,6 +193,8 @@ public static function changePassword(User $user, string $oldPassword, string $n $newHash = Encryption::passwordHash($newPassword, $newSalt); Factory::getUserFactory()->mset($user, [User::PASSWORD_HASH => $newHash, User::PASSWORD_SALT => $newSalt, User::IS_COMPUTED_PASSWORD => 0]); + // The old password can no longer be used to log in, so sessions resting on it should not survive either + RefreshTokenUtils::revokeAllForUser($user->getId()); } /** @@ -211,6 +217,7 @@ public static function setPassword(int $userId, string $password, User $adminUse $newHash = Encryption::passwordHash($password, $newSalt); Factory::getUserFactory()->mset($user, [User::PASSWORD_HASH => $newHash, User::PASSWORD_SALT => $newSalt, User::IS_COMPUTED_PASSWORD => 0]); + RefreshTokenUtils::revokeAllForUser($user->getId()); } /** diff --git a/src/migrations/mysql/20260914090000_refresh-token.sql b/src/migrations/mysql/20260914090000_refresh-token.sql new file mode 100644 index 000000000..730d44a20 --- /dev/null +++ b/src/migrations/mysql/20260914090000_refresh-token.sql @@ -0,0 +1,19 @@ +-- Refresh tokens: long-lived, single-use credentials that are exchanged for short-lived access tokens. +-- Only the SHA-256 hash of the token string is stored. Tokens rotate on every use; all rotations of one +-- login session share a familyId so a replayed token can revoke the entire session. +CREATE TABLE `RefreshToken` ( + `refreshTokenId` int NOT NULL AUTO_INCREMENT, + `userId` int NOT NULL, + `tokenHash` varchar(64) NOT NULL, + `familyId` varchar(32) NOT NULL, + `issuedAt` bigint NOT NULL, + `endValid` bigint NOT NULL, + `usedAt` bigint DEFAULT NULL, + `isRevoked` tinyint(1) NOT NULL DEFAULT '0', + PRIMARY KEY (`refreshTokenId`), + UNIQUE KEY `uq_refreshToken_tokenHash` (`tokenHash`), + KEY `idx_refreshToken_userId` (`userId`), + KEY `idx_refreshToken_familyId` (`familyId`), + KEY `idx_refreshToken_endValid` (`endValid`), + CONSTRAINT `fk_refreshToken_user` FOREIGN KEY (`userId`) REFERENCES `htp_User` (`userId`) +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_ai_ci; diff --git a/src/migrations/postgres/20260914090000_refresh-token.sql b/src/migrations/postgres/20260914090000_refresh-token.sql new file mode 100644 index 000000000..2c2e46625 --- /dev/null +++ b/src/migrations/postgres/20260914090000_refresh-token.sql @@ -0,0 +1,19 @@ +-- Refresh tokens: long-lived, single-use credentials that are exchanged for short-lived access tokens. +-- Only the SHA-256 hash of the token string is stored. Tokens rotate on every use; all rotations of one +-- login session share a familyid so a replayed token can revoke the entire session. +CREATE TABLE refreshtoken ( + refreshtokenid integer GENERATED ALWAYS AS IDENTITY PRIMARY KEY, + userid integer NOT NULL, + tokenhash varchar(64) NOT NULL, + familyid varchar(32) NOT NULL, + issuedat bigint NOT NULL, + endvalid bigint NOT NULL, + usedat bigint DEFAULT NULL, + isrevoked boolean DEFAULT false NOT NULL, + CONSTRAINT uq_refreshtoken_tokenhash UNIQUE (tokenhash), + CONSTRAINT refreshtoken_user_fkey FOREIGN KEY (userid) REFERENCES htp_user(userid) +); + +CREATE INDEX refreshtoken_userid_idx ON refreshtoken(userid); +CREATE INDEX refreshtoken_familyid_idx ON refreshtoken(familyid); +CREATE INDEX refreshtoken_endvalid_idx ON refreshtoken(endvalid);