Hello,
-An XNAT account has been created at trust {{trust_name}}. You have been granted access to project: {{project_name}} (ID: {{project_id}}).
+An XNAT account has been created for you at trust {{trust_name}}. You have been granted access to project: {{project_name}} (ID: {{project_id}}).
- +Your XNAT credentials:
+Set your password to activate your account:
| Username: | {{username}} | ||
| Password: | -{{password}} | +Set-password link: | +{{setup_path}} |
Please change your password when you first log in.
+Open your trust's XNAT in a web browser, then append the set-password link path above to its address and go to it. You'll be asked to choose your own password — no password is ever sent by email.
+ + +Important: XNAT is only reachable from inside {{trust_name}}'s secure network. Open this link from a machine connected to that network (for example, using the connection you normally use to reach XNAT). It cannot be opened over the public internet. The link can be used once and expires.
+This is an automated message. Please do not reply to this email. This message may contain confidential information. If you are not the intended recipient, please do not disclose, copy, or distribute it.
diff --git a/deploy/providers/AWS/templates/ses/flip-xnat-credentials.txt b/deploy/providers/AWS/templates/ses/flip-xnat-invite.txt similarity index 58% rename from deploy/providers/AWS/templates/ses/flip-xnat-credentials.txt rename to deploy/providers/AWS/templates/ses/flip-xnat-invite.txt index 5651e4d69..37370970a 100644 --- a/deploy/providers/AWS/templates/ses/flip-xnat-credentials.txt +++ b/deploy/providers/AWS/templates/ses/flip-xnat-invite.txt @@ -14,13 +14,15 @@ FLIP - Federated Learning & Interoperability Platform Hello, -An XNAT account has been created at trust {{trust_name}}. You have been granted access to project: {{project_name}} (ID: {{project_id}}). +An XNAT account has been created for you at trust {{trust_name}}. You have been granted access to project: {{project_name}} (ID: {{project_id}}). -Your XNAT credentials: +Set your password to activate your account: Username: {{username}} - Password: {{password}} + Set-password link: {{setup_path}} -Please change your password when you first log in. +Open your trust's XNAT in a web browser, then append the set-password link path above to its address and go to it. You'll be asked to choose your own password - no password is ever sent by email. + +IMPORTANT: XNAT is only reachable from inside {{trust_name}}'s secure network. Open this link from a machine connected to that network (for example, using the connection you normally use to reach XNAT). It cannot be opened over the public internet. The link can be used once and expires. This is an automated message. Please do not reply to this email. This message may contain confidential information. If you are not the intended recipient, please do not disclose, copy, or distribute it. diff --git a/deploy/providers/AWS/tests/test_email_templates.py b/deploy/providers/AWS/tests/test_email_templates.py index 61af89f06..7b15cef81 100644 --- a/deploy/providers/AWS/tests/test_email_templates.py +++ b/deploy/providers/AWS/tests/test_email_templates.py @@ -57,7 +57,7 @@ class TestSesData: project_name: str = "Brain Tumour Segmentation" project_id: str = "BTS-001" username: str = "jdoe" - password: str = "X7k!mP2$vR9n" + setup_path: str = "/app/template/XDATScreen_UpdateUser.vm?a=alias-123&s=secret-abc" class EmailTemplateTester: @@ -83,7 +83,7 @@ def __init__(self, test_user: TestUser | None = None, test_ses: TestSesData | No # Load SES templates ses_dir = aws_dir / "templates" / "ses" self.ACCESS_REQUEST_TEMPLATE_HTML = (ses_dir / "flip-access-request.html").read_text() - self.XNAT_CREDENTIALS_TEMPLATE_HTML = (ses_dir / "flip-xnat-credentials.html").read_text() + self.XNAT_INVITE_TEMPLATE_HTML = (ses_dir / "flip-xnat-invite.html").read_text() def substitute_placeholders(self, template: str) -> tuple[str, dict[str, str]]: """ @@ -112,7 +112,7 @@ def substitute_placeholders(self, template: str) -> tuple[str, dict[str, str]]: "{{project_name}}": self.test_ses.project_name, "{{project_id}}": self.test_ses.project_id, "{{username}}": self.test_ses.username, - "{{password}}": self.test_ses.password, + "{{setup_path}}": self.test_ses.setup_path, } rendered = template @@ -192,7 +192,7 @@ def render_and_save(self, output_dir: Path) -> None: ("password_reset_code", self.PASSWORD_RESET_CODE_TEMPLATE_HTML), ("password_reset_link", self.PASSWORD_RESET_LINK_TEMPLATE_HTML), ("access_request", self.ACCESS_REQUEST_TEMPLATE_HTML), - ("xnat_credentials", self.XNAT_CREDENTIALS_TEMPLATE_HTML), + ("xnat_invite", self.XNAT_INVITE_TEMPLATE_HTML), ] for name, template in templates: @@ -261,7 +261,7 @@ def test_all(self) -> list[dict]: ("Password Reset (Code)", self.PASSWORD_RESET_CODE_TEMPLATE_HTML), ("Password Reset (Link)", self.PASSWORD_RESET_LINK_TEMPLATE_HTML), ("Access Request", self.ACCESS_REQUEST_TEMPLATE_HTML), - ("XNAT Credentials", self.XNAT_CREDENTIALS_TEMPLATE_HTML), + ("XNAT Invite", self.XNAT_INVITE_TEMPLATE_HTML), ] results = [] @@ -353,7 +353,7 @@ def main(): if args.serve: print(f"\n📧 Starting local HTTP server at http://localhost:{args.port}") print("Open the following URLs in your browser:") - for name in ["invite", "password_reset_code", "password_reset_link", "access_request", "xnat_credentials"]: + for name in ["invite", "password_reset_code", "password_reset_link", "access_request", "xnat_invite"]: print(f" • http://localhost:{args.port}/flip_email_{name}.html") print("\nPress Ctrl+C to stop the server.\n") diff --git a/docs/source/components/component-xnat.rst b/docs/source/components/component-xnat.rst index 2fa9b50e4..4e461ecff 100644 --- a/docs/source/components/component-xnat.rst +++ b/docs/source/components/component-xnat.rst @@ -16,16 +16,14 @@ XNAT UI .. _receiving-xnat-credentials: -Receiving XNAT Account Credentials -================================== +Receiving XNAT Account Access +============================= -On approval of a FLIP project, any associated users will be granted access to the respective XNAT project at each trust. New XNAT user accounts will be generated as necessary. The email address associated with the FLIP user account will be sent details of their XNAT account credentials pertaining to each participating trust. +On approval of a FLIP project, any associated users are granted access to the respective XNAT project at each participating trust, and new XNAT user accounts are created as necessary. -.. figure:: ../assets/xnat/credentials_email.png - :width: 500 - :align: center +If a new XNAT account is created for you, the email address associated with your FLIP account receives a one-time link to set your own password for that trust's XNAT — no password is ever sent by email. Because XNAT is only reachable from inside the trust's secure network (see `Access`_ below), open the link from a machine on that network; it can be used once and expires. - Email sent with XNAT account credentials (password masked). +If you already have an XNAT account at a trust and are later added to a further project there, you instead receive a notification that you have been added to the project — log in with your existing credentials. Access ====== diff --git a/flip-api/src/flip_api/domain/interfaces/trust.py b/flip-api/src/flip_api/domain/interfaces/trust.py index 970cfb33b..fa314da6d 100644 --- a/flip-api/src/flip_api/domain/interfaces/trust.py +++ b/flip-api/src/flip_api/domain/interfaces/trust.py @@ -115,10 +115,14 @@ class ICreateImagingProject(BaseModel): class ICreatedImagingUser(BaseModel): - """Represents a user created on XNAT. Used to be called IImageUser in the old repo.""" + """Represents a user created on XNAT. Used to be called IImageUser in the old repo. + + Carries the AES-encrypted, host-less setup path (an XNAT alias-token link) the new user follows + to set their own password — never a password. See FLIP-PT-079. + """ username: str - encrypted_password: str + encrypted_setup_path: str email: EmailStr @@ -139,11 +143,13 @@ class ICreatedImagingProject(BaseModel): class ISesTemplateData(BaseModel): + """Template data for the XNAT invite email: a host-less setup link, never a password (FLIP-PT-079).""" + trust_name: str project_name: str project_id: UUID username: str - password: str + setup_path: str class ISesProjectAccessTemplateData(BaseModel): diff --git a/flip-api/src/flip_api/private_services/imaging_notifications.py b/flip-api/src/flip_api/private_services/imaging_notifications.py index 72b7809fd..fb3b13405 100644 --- a/flip-api/src/flip_api/private_services/imaging_notifications.py +++ b/flip-api/src/flip_api/private_services/imaging_notifications.py @@ -25,7 +25,7 @@ ISesTemplateData, ) from flip_api.private_services.project_images_helpers import insert_status -from flip_api.utils.constants import IMAGING_CREDENTIALS_TEMPLATE_NAME, IMAGING_PROJECT_ACCESS_TEMPLATE_NAME +from flip_api.utils.constants import IMAGING_INVITE_TEMPLATE_NAME, IMAGING_PROJECT_ACCESS_TEMPLATE_NAME from flip_api.utils.email_sender import EmailDispatchError, send_templated_email from flip_api.utils.encryption import decrypt from flip_api.utils.logger import logger @@ -45,7 +45,7 @@ def handle_imaging_task_completed(task: TrustTask, db: Session) -> None: any recipient, so it aborts the run. Both callers (``trust_tasks`` and the ``stale_task_recovery`` sweep) clear ``needs_post_processing`` only on a clean return, so swallowing that would discard the retry and leave those - users with no credentials and no trace in the retry queue. Anything else, + users with no invite and no trace in the retry queue. Anything else, notably a ``ClientError`` such as SES ``MessageRejected`` on one bad address, is that recipient's problem alone: it is logged and skipped so the remaining recipients still get their mail and the task reaches a terminal @@ -104,27 +104,27 @@ def handle_imaging_task_completed(task: TrustTask, db: Session) -> None: trust = db.exec(select(Trust).where(Trust.id == task.trust_id)).first() trust_name = trust.name if trust else "Unknown Trust" - # Send credential emails to newly created users + # Send invite emails to newly created users. The email carries a host-less "set your own + # password" link (an XNAT alias-token path), never a password (FLIP-PT-079). The link is + # decrypted here only to place it in the email — no standing credential is ever transmitted. for user in imaging_project.created_users: try: - decrypted_password = decrypt(user.encrypted_password) + setup_path = decrypt(user.encrypted_setup_path) template_data = ISesTemplateData( trust_name=trust_name, project_name=imaging_project.name, project_id=project_id, username=user.username, - password=decrypted_password, + setup_path=setup_path, ) send_templated_email( recipient=user.email, - template_name=IMAGING_CREDENTIALS_TEMPLATE_NAME, + template_name=IMAGING_INVITE_TEMPLATE_NAME, template_data=template_data.model_dump(mode="json"), ) - logger.info( - f"XNAT credentials notification dispatched to {user.email} for project '{imaging_project.name}'" - ) + logger.info(f"XNAT invite dispatched to {user.email} for project '{imaging_project.name}'") except BotoCoreError as e: raise EmailDispatchError( @@ -135,7 +135,7 @@ def handle_imaging_task_completed(task: TrustTask, db: Session) -> None: except Exception as e: # exception(), not error(): the traceback is what distinguishes a # rejected address from a template or decrypt fault. - logger.exception(f"Failed to send credentials email to {user.email}: {e}") + logger.exception(f"Failed to send invite email to {user.email}: {e}") # Send project access notifications to existing users (no password) for added_user in imaging_project.added_users: diff --git a/flip-api/src/flip_api/utils/constants.py b/flip-api/src/flip_api/utils/constants.py index 526c9cf9a..603aade3d 100644 --- a/flip-api/src/flip_api/utils/constants.py +++ b/flip-api/src/flip_api/utils/constants.py @@ -22,7 +22,8 @@ # AWS SES email templates ACCESS_REQUEST_TEMPLATE_NAME = "flip-access-request" -IMAGING_CREDENTIALS_TEMPLATE_NAME = "flip-xnat-credentials" +# Invite email carrying a host-less "set your own password" link — never a password (FLIP-PT-079). +IMAGING_INVITE_TEMPLATE_NAME = "flip-xnat-invite" IMAGING_PROJECT_ACCESS_TEMPLATE_NAME = "flip-xnat-added-to-project" diff --git a/flip-api/src/flip_api/utils/email_sender.py b/flip-api/src/flip_api/utils/email_sender.py index a3bb4c328..f40f7b080 100644 --- a/flip-api/src/flip_api/utils/email_sender.py +++ b/flip-api/src/flip_api/utils/email_sender.py @@ -22,14 +22,20 @@ from flip_api.utils.logger import logger # Substrings marking a template-data key whose value must never reach the -# logs. The console backend logs the payload, and flip-xnat-credentials -# carries the user's decrypted XNAT password. Matched as substrings rather +# logs. The console backend logs the payload, and flip-xnat-invite carries the +# user's decrypted XNAT alias-token path. Matched as substrings rather # than exact names, and at every depth of the payload: a denylist guarding a # live credential has to fail safe, so a future `temp_password`, `api_secret` # or `access_token` field is redacted without anyone remembering to update # this set, and without depending on the payload staying flat the way today's # ISesTemplateData shapes happen to be. -_REDACTED_KEY_MARKERS = ("password", "secret", "token", "credential") +# +# `setup_path` is here because it is the one credential-bearing field whose +# name says nothing about it: FLIP-PT-079 replaced the emailed password with a +# one-time set-password link, and its `?a=…&s=…` alias-token pair is a bearer +# capability over that XNAT account. Nothing in "setup_path" matches the +# markers above, so without this entry the invite link is logged in full. +_REDACTED_KEY_MARKERS = ("password", "secret", "token", "credential", "setup_path") # Cap on a single logged template value. reason_for_access arrives on the # unauthenticated POST /users/access with no length bound. diff --git a/flip-api/tests/integration/test_console_email_backend.py b/flip-api/tests/integration/test_console_email_backend.py index dc7b253cb..6b4c68795 100644 --- a/flip-api/tests/integration/test_console_email_backend.py +++ b/flip-api/tests/integration/test_console_email_backend.py @@ -34,12 +34,14 @@ from flip_api.private_services.imaging_notifications import handle_imaging_task_completed from flip_api.utils.constants import ( ACCESS_REQUEST_TEMPLATE_NAME, - IMAGING_CREDENTIALS_TEMPLATE_NAME, + IMAGING_INVITE_TEMPLATE_NAME, IMAGING_PROJECT_ACCESS_TEMPLATE_NAME, ) from flip_api.utils.encryption import encrypt -XNAT_PASSWORD = "hunter2-the-password" # pragma: allowlist secret +# The invite link PT-079 sends in place of a password: an XNAT alias-token path, whose +# a=/s= pair is a bearer capability to set that user's password. +XNAT_SETUP_PATH = "/app/template/XDATScreen_UpdateUser.vm?a=alias123&s=tok3nva1ue" # pragma: allowlist secret @pytest.fixture(autouse=True) @@ -78,10 +80,10 @@ def test_request_access_succeeds_and_logs_instead_of_sending(client: TestClient, assert persisted.email_notified is True -def test_imaging_notifications_log_both_templates_without_leaking_the_password( +def test_imaging_notifications_log_both_templates_without_leaking_the_invite_link( session, trust_factory, project_factory, caplog ): - """Both XNAT emails are logged, and the decrypted password never is.""" + """Both XNAT emails are logged, and the decrypted invite link never is.""" trust = trust_factory() project = project_factory() session.add(trust) @@ -103,7 +105,7 @@ def test_imaging_notifications_log_both_templates_without_leaking_the_password( "created_users": [ { "username": "newbie@example.com", - "encrypted_password": encrypt(XNAT_PASSWORD), + "encrypted_setup_path": encrypt(XNAT_SETUP_PATH), "email": "newbie@example.com", } ], @@ -117,8 +119,9 @@ def test_imaging_notifications_log_both_templates_without_leaking_the_password( with caplog.at_level(logging.INFO, logger="uvicorn"): handle_imaging_task_completed(task, session) - assert IMAGING_CREDENTIALS_TEMPLATE_NAME in caplog.text + assert IMAGING_INVITE_TEMPLATE_NAME in caplog.text assert IMAGING_PROJECT_ACCESS_TEMPLATE_NAME in caplog.text - # The credentials template carries the user's decrypted XNAT password. - # Logging it would turn a dev convenience into a credential leak. - assert XNAT_PASSWORD not in caplog.text + # The invite template carries the user's decrypted alias-token path, which is a + # bearer capability to set their password. Logging it would turn a dev convenience + # into a credential leak, exactly as an emailed password would have. + assert XNAT_SETUP_PATH not in caplog.text diff --git a/flip-api/tests/integration/test_ses_round_trips.py b/flip-api/tests/integration/test_ses_round_trips.py index 6f5081557..6dc84a1a7 100644 --- a/flip-api/tests/integration/test_ses_round_trips.py +++ b/flip-api/tests/integration/test_ses_round_trips.py @@ -46,7 +46,7 @@ from flip_api.private_services.imaging_notifications import handle_imaging_task_completed from flip_api.utils.constants import ( ACCESS_REQUEST_TEMPLATE_NAME, - IMAGING_CREDENTIALS_TEMPLATE_NAME, + IMAGING_INVITE_TEMPLATE_NAME, IMAGING_PROJECT_ACCESS_TEMPLATE_NAME, ) from flip_api.utils.encryption import encrypt @@ -110,23 +110,23 @@ def _seed_completed_imaging_task(session, trust_id: UUID, project_id: UUID) -> T """Create a CREATE_IMAGING task with a populated result block. Result mirrors what trust-api returns when an XNAT project is created: - one newly created user (gets a credentials email) and one already- - existing user being added to the project (gets a project-access + one newly created user (gets an invite email with a set-password link) and + one already-existing user being added to the project (gets a project-access notification). """ - encrypted_password = encrypt("hunter2-the-password") # pragma: allowlist secret + encrypted_setup_path = encrypt("/app/template/XDATScreen_UpdateUser.vm?a=alias&s=secret") payload = {"project_id": str(project_id)} # The result schema is the trust-side ``ICreatedImagingProject``, which the # parser deserialises with ``ID`` -> ``imaging_project_id``. Both # ``created_users`` and ``added_users`` need a structured user shape with - # email + (encrypted) password where applicable. + # email + (encrypted) setup path where applicable. result = { "ID": str(uuid4()), "name": "ICU-Imaging-Project", "created_users": [ { "username": "newbie@example.com", - "encrypted_password": encrypted_password, + "encrypted_setup_path": encrypted_setup_path, "email": "newbie@example.com", } ], @@ -164,7 +164,7 @@ def test_handle_imaging_task_sends_one_email_per_user_and_persists_status( handle_imaging_task_completed(task, session) template_names = [c["Content"]["Template"]["TemplateName"] for c in ses_send_email_recorder] - assert template_names.count(IMAGING_CREDENTIALS_TEMPLATE_NAME) == 1 + assert template_names.count(IMAGING_INVITE_TEMPLATE_NAME) == 1 assert template_names.count(IMAGING_PROJECT_ACCESS_TEMPLATE_NAME) == 1 # Status row: persisted exactly once. diff --git a/flip-api/tests/unit/private_services/test_imaging_notifications.py b/flip-api/tests/unit/private_services/test_imaging_notifications.py index fd0f8c681..36d403f15 100644 --- a/flip-api/tests/unit/private_services/test_imaging_notifications.py +++ b/flip-api/tests/unit/private_services/test_imaging_notifications.py @@ -74,8 +74,8 @@ def mock_insert_status(): def test_sends_email_to_each_created_user(mock_send_email, mock_decrypt, mock_insert_status): """Should send one SES email per created user with correct template data.""" users = [ - {"username": "user1", "encrypted_password": "enc1", "email": "user1@test.com"}, # pragma: allowlist secret - {"username": "user2", "encrypted_password": "enc2", "email": "user2@test.com"}, # pragma: allowlist secret + {"username": "user1", "encrypted_setup_path": "enc1", "email": "user1@test.com"}, + {"username": "user2", "encrypted_setup_path": "enc2", "email": "user2@test.com"}, ] task = _make_task(users) @@ -98,7 +98,7 @@ def test_sends_email_to_each_created_user(mock_send_email, mock_decrypt, mock_in assert template_data["trust_name"] == "Trust_1" assert template_data["project_name"] == "Test Imaging Project" assert template_data["username"] == "user1" - assert template_data["password"] == "decrypted_enc1" # pragma: allowlist secret + assert template_data["setup_path"] == "decrypted_enc1" # Verify second user's email second_call = mock_send_email.call_args_list[1] @@ -110,7 +110,7 @@ def test_inserts_xnat_project_status(mock_send_email, mock_decrypt, mock_insert_ from flip_api.db.models.main_models import XNATImageStatus users = [ - {"username": "user1", "encrypted_password": "enc1", "email": "user1@test.com"}, # pragma: allowlist secret + {"username": "user1", "encrypted_setup_path": "enc1", "email": "user1@test.com"}, ] task = _make_task(users) @@ -139,7 +139,7 @@ def test_inserts_xnat_project_status(mock_send_email, mock_decrypt, mock_insert_ def test_inserts_status_with_no_query(mock_send_email, mock_decrypt, mock_insert_status): """Should pass query_id=None when project has no queries.""" users = [ - {"username": "user1", "encrypted_password": "enc1", "email": "user1@test.com"}, # pragma: allowlist secret + {"username": "user1", "encrypted_setup_path": "enc1", "email": "user1@test.com"}, ] task = _make_task(users) @@ -185,8 +185,8 @@ def test_systemic_failure_raises_so_the_task_stays_retryable(mock_send_email, mo cannot be constructed cannot send to anyone. """ users = [ - {"username": "user1", "encrypted_password": "enc1", "email": "user1@test.com"}, # pragma: allowlist secret - {"username": "user2", "encrypted_password": "enc2", "email": "user2@test.com"}, # pragma: allowlist secret + {"username": "user1", "encrypted_setup_path": "enc1", "email": "user1@test.com"}, + {"username": "user2", "encrypted_setup_path": "enc2", "email": "user2@test.com"}, ] task = _make_task(users, added_users=[{"username": "existing1", "email": "existing1@test.com"}]) @@ -216,7 +216,7 @@ def test_single_recipient_rejection_does_not_raise(mock_send_email, mock_decrypt task never reaching a terminal state. """ task = _make_task([ - {"username": "user1", "encrypted_password": "enc1", "email": "typo@test.com"}, # pragma: allowlist secret + {"username": "user1", "encrypted_setup_path": "enc1", "email": "typo@test.com"}, ]) mock_db = MagicMock() @@ -242,7 +242,7 @@ def test_single_recipient_systemic_failure_raises(mock_send_email, mock_decrypt, the outcome is decided by the exception type rather than the count. """ task = _make_task([ - {"username": "user1", "encrypted_password": "enc1", "email": "user1@test.com"}, # pragma: allowlist secret + {"username": "user1", "encrypted_setup_path": "enc1", "email": "user1@test.com"}, ]) mock_db = MagicMock() @@ -278,8 +278,8 @@ def test_added_user_systemic_failure_raises(mock_send_email, mock_decrypt, mock_ def test_partial_failure_does_not_raise(mock_send_email, mock_decrypt, mock_insert_status): """One bad address is not systemic: the run succeeds so the task is not retried forever.""" users = [ - {"username": "user1", "encrypted_password": "enc1", "email": "user1@test.com"}, # pragma: allowlist secret - {"username": "user2", "encrypted_password": "enc2", "email": "user2@test.com"}, # pragma: allowlist secret + {"username": "user1", "encrypted_setup_path": "enc1", "email": "user1@test.com"}, + {"username": "user2", "encrypted_setup_path": "enc2", "email": "user2@test.com"}, ] task = _make_task(users) @@ -300,8 +300,8 @@ def test_partial_failure_does_not_raise(mock_send_email, mock_decrypt, mock_inse def test_ses_failure_for_one_user_continues_to_next(mock_send_email, mock_decrypt, mock_insert_status): """Should continue sending to remaining users if the send fails for one.""" users = [ - {"username": "user1", "encrypted_password": "enc1", "email": "user1@test.com"}, # pragma: allowlist secret - {"username": "user2", "encrypted_password": "enc2", "email": "user2@test.com"}, # pragma: allowlist secret + {"username": "user1", "encrypted_setup_path": "enc1", "email": "user1@test.com"}, + {"username": "user2", "encrypted_setup_path": "enc2", "email": "user2@test.com"}, ] task = _make_task(users) @@ -322,8 +322,8 @@ def test_ses_failure_for_one_user_continues_to_next(mock_send_email, mock_decryp def test_decryption_failure_continues_to_next_user(mock_send_email, mock_insert_status): """Should continue to next user if decryption fails for one.""" users = [ - {"username": "user1", "encrypted_password": "enc1", "email": "user1@test.com"}, # pragma: allowlist secret - {"username": "user2", "encrypted_password": "enc2", "email": "user2@test.com"}, # pragma: allowlist secret + {"username": "user1", "encrypted_setup_path": "enc1", "email": "user1@test.com"}, + {"username": "user2", "encrypted_setup_path": "enc2", "email": "user2@test.com"}, ] task = _make_task(users) @@ -387,7 +387,7 @@ def test_missing_id_field_raises_value_error(): def test_sends_project_access_email_to_added_users(mock_send_email, mock_decrypt, mock_insert_status): - """Should send project access emails (no password) to existing users added to the project.""" + """Should send project access emails (no invite link) to existing users added to the project.""" added_users = [ {"username": "existing1", "email": "existing1@test.com"}, {"username": "existing2", "email": "existing2@test.com"}, @@ -405,7 +405,7 @@ def test_sends_project_access_email_to_added_users(mock_send_email, mock_decrypt assert mock_send_email.call_count == 2 - # Verify correct template is used (not credentials template) + # Verify correct template is used (not the invite template) first_call = mock_send_email.call_args_list[0] assert first_call.kwargs["recipient"] == "existing1@test.com" assert first_call.kwargs["template_name"] == "flip-xnat-added-to-project" @@ -414,13 +414,13 @@ def test_sends_project_access_email_to_added_users(mock_send_email, mock_decrypt assert template_data["trust_name"] == "Trust_1" assert template_data["project_name"] == "Test Imaging Project" assert template_data["username"] == "existing1" - assert "password" not in template_data + assert "setup_path" not in template_data -def test_sends_both_credential_and_access_emails(mock_send_email, mock_decrypt, mock_insert_status): - """Should send credential emails to created users AND access emails to added users.""" +def test_sends_both_invite_and_access_emails(mock_send_email, mock_decrypt, mock_insert_status): + """Should send invite emails to created users AND access emails to added users.""" created_users = [ - {"username": "new1", "encrypted_password": "enc1", "email": "new1@test.com"}, # pragma: allowlist secret + {"username": "new1", "encrypted_setup_path": "enc1", "email": "new1@test.com"}, ] added_users = [ {"username": "existing1", "email": "existing1@test.com"}, @@ -438,9 +438,9 @@ def test_sends_both_credential_and_access_emails(mock_send_email, mock_decrypt, assert mock_send_email.call_count == 2 - # First call: credentials email to new user + # First call: invite email to new user cred_call = mock_send_email.call_args_list[0] - assert cred_call.kwargs["template_name"] == "flip-xnat-credentials" + assert cred_call.kwargs["template_name"] == "flip-xnat-invite" assert cred_call.kwargs["recipient"] == "new1@test.com" # Second call: access email to existing user diff --git a/flip-api/tests/unit/trusts_services/test_start_project_imaging_creation.py b/flip-api/tests/unit/trusts_services/test_start_project_imaging_creation.py index b2bfd1d25..8a59717af 100644 --- a/flip-api/tests/unit/trusts_services/test_start_project_imaging_creation.py +++ b/flip-api/tests/unit/trusts_services/test_start_project_imaging_creation.py @@ -33,8 +33,7 @@ user_id = uuid.uuid4() user_name = "user one" user_email = "user1@example.com" -# pragma: allowlist nextline secret -user_encrypted_password = "encrypted_pw" +user_encrypted_setup_path = "encrypted_setup_path" user_pool_id = uuid.uuid4() # ============================================================================================= diff --git a/trust/imaging-api/imaging_api/routers/schemas.py b/trust/imaging-api/imaging_api/routers/schemas.py index 421fb443a..8c5380a72 100644 --- a/trust/imaging-api/imaging_api/routers/schemas.py +++ b/trust/imaging-api/imaging_api/routers/schemas.py @@ -66,10 +66,14 @@ class CreateUser(BaseModel): class CreatedUser(BaseModel): - """Represents a user created on XNAT.""" + """Represents a user created on XNAT. + + Carries the AES-encrypted, host-less setup path (an XNAT alias-token link) the new user + follows to set their own password — never a password. See FLIP-PT-079. + """ username: str - encrypted_password: str + encrypted_setup_path: str email: EmailStr diff --git a/trust/imaging-api/imaging_api/services/users.py b/trust/imaging-api/imaging_api/services/users.py index 698927c52..0d31cdee4 100644 --- a/trust/imaging-api/imaging_api/services/users.py +++ b/trust/imaging-api/imaging_api/services/users.py @@ -11,6 +11,7 @@ # import re +import urllib.parse import requests @@ -23,6 +24,12 @@ XNAT_URL = get_settings().XNAT_URL +# Host-less "set your own password" path a newly created XNAT user follows. It is deliberately +# host-less: XNAT is only reachable from inside the trust enclave, so the hub emails this path and +# tells the recipient to open it against their own XNAT address (FLIP-PT-079). `a`/`s` are the XNAT +# alias-token alias + secret; visiting the link authenticates the user and shows a password form. +XNAT_SETUP_PATH_TEMPLATE = "/app/template/XDATScreen_UpdateUser.vm?a={alias}&s={secret}" + def get_xnat_users(headers: dict[str, str]) -> list[User]: """ @@ -151,12 +158,49 @@ def user_exists(username: str, headers: dict[str, str]) -> bool: return False +def issue_setup_token(username: str, headers: dict[str, str]) -> str: + """Mint a single-use XNAT alias token for ``username`` and build the host-less setup path. + + The trust's service account (a site admin) issues the token on the user's behalf via XNAT's + ``GET /data/services/tokens/issue/user/{username}`` endpoint. Visiting the returned path + authenticates the user and presents a "set your own password" form; XNAT invalidates the token + once the password is set. The path is host-less on purpose — XNAT is only reachable from inside + the trust enclave, so the hub emails the path and tells the recipient to open it against their + own XNAT address rather than emailing a password (FLIP-PT-079). + + Args: + username (str): XNAT username to issue the setup token for. + headers (dict[str, str]): XNAT authentication headers (service-account session). + + Returns: + str: The host-less setup path, e.g. ``/app/template/XDATScreen_UpdateUser.vm?a=…&s=…``. + + Raises: + Exception: If XNAT returns a non-200 response when issuing the token. + """ + quoted_username = urllib.parse.quote(username, safe="") + response = requests.get(f"{XNAT_URL}/data/services/tokens/issue/user/{quoted_username}", headers=headers) + if response.status_code != 200: + raise Exception(f"Error: XNAT setup-token issuance failed: {response.status_code} - {response.text}") + + token = response.json() + alias = urllib.parse.quote(token["alias"], safe="") + secret = urllib.parse.quote(token["secret"], safe="") + return XNAT_SETUP_PATH_TEMPLATE.format(alias=alias, secret=secret) + + def create_user_from_central_hub_user( central_hub_user: CentralHubUser, headers: dict[str, str] ) -> tuple[CreatedUser, User]: """ Convert central hub user to XNAT CreateUser request object, and create user on XNAT. + The user is created with a throwaway random password that is never disclosed; instead of + emailing a password (FLIP-PT-079) we mint a single-use setup token and return the host-less + link the user follows to set their own password directly in XNAT. The random password is + load-bearing: XNAT's password-change path raises on a user that has no existing password, so + the account must be created *with* a password, not passwordless. + Args: central_hub_user (imaging_api.routers.schemas.CentralHubUser): The user's details on the Central Hub. headers (dict[str, str]): XNAT authentication headers. @@ -168,9 +212,10 @@ def create_user_from_central_hub_user( create_user_request = to_create_imaging_user(central_hub_user, headers) # Actually create user_profile = create_user(create_user_request, headers) + setup_path = issue_setup_token(user_profile.username, headers) created_user = CreatedUser( username=user_profile.username, - encrypted_password=encrypt(create_user_request.password), + encrypted_setup_path=encrypt(setup_path), email=create_user_request.email, ) return created_user, user_profile diff --git a/trust/imaging-api/tests/services/test_projects.py b/trust/imaging-api/tests/services/test_projects.py index cc30c7251..fa9ae247c 100644 --- a/trust/imaging-api/tests/services/test_projects.py +++ b/trust/imaging-api/tests/services/test_projects.py @@ -739,7 +739,7 @@ def test_add_central_hub_users_existing_user(mock_get_profile, mock_add, headers def test_add_central_hub_users_new_user(mock_get_profile, mock_create, mock_add, headers): mock_get_profile.side_effect = NotFoundError("not found") user_profile = User(**_USER_DICT) - created_user = CreatedUser(username="alice", encrypted_password="enc", email="alice@test.com") + created_user = CreatedUser(username="alice", encrypted_setup_path="enc", email="alice@test.com") mock_create.return_value = (created_user, user_profile) mock_add.return_value = user_profile diff --git a/trust/imaging-api/tests/services/test_users.py b/trust/imaging-api/tests/services/test_users.py index 52f0d0bcf..45be48eea 100644 --- a/trust/imaging-api/tests/services/test_users.py +++ b/trust/imaging-api/tests/services/test_users.py @@ -23,6 +23,7 @@ create_user_from_central_hub_user, get_user_profile_by, get_xnat_users, + issue_setup_token, to_create_imaging_user, user_exists, ) @@ -269,7 +270,7 @@ def test_create_user_success(mock_post, mock_get_profile, headers): mock_get_profile.return_value = User(**_SAMPLE_USER_DICT) user_req = CreateUser( - username="alice", password="pass", firstName="Alice", + username="alice", password="pass", firstName="Alice", # pragma: allowlist secret lastName="A", email="alice@test.com", ) profile = create_user(user_req, headers) @@ -281,7 +282,7 @@ def test_create_user_conflict(mock_post, headers): mock_post.return_value = MagicMock(status_code=409, text="conflict") user_req = CreateUser( - username="alice", password="pass", firstName="Alice", + username="alice", password="pass", firstName="Alice", # pragma: allowlist secret lastName="A", email="alice@test.com", ) with pytest.raises(AlreadyExistsError, match="already exists"): @@ -293,22 +294,48 @@ def test_create_user_server_error(mock_post, headers): mock_post.return_value = MagicMock(status_code=500, text="Server Error") user_req = CreateUser( - username="alice", password="pass", firstName="Alice", + username="alice", password="pass", firstName="Alice", # pragma: allowlist secret lastName="A", email="alice@test.com", ) with pytest.raises(Exception, match="XNAT user creation failed"): create_user(user_req, headers) +# --------------------------------------------------------------------------- +# issue_setup_token +# --------------------------------------------------------------------------- +@patch("imaging_api.services.users.requests.get") +def test_issue_setup_token_builds_hostless_path(mock_get, headers): + """Mints an alias token and returns a host-less set-password path with url-encoded secret.""" + mock_get.return_value = MagicMock( + status_code=200, + json=lambda: {"alias": "al-123", "secret": "sek ret"}, # pragma: allowlist secret + ) + + path = issue_setup_token("alice", headers) + + # No scheme/host leaks into the emailed link (XNAT is enclave-only — FLIP-PT-079); secret is encoded. + assert path == "/app/template/XDATScreen_UpdateUser.vm?a=al-123&s=sek%20ret" + assert not path.startswith("http") + + +@patch("imaging_api.services.users.requests.get") +def test_issue_setup_token_raises_on_error(mock_get, headers): + mock_get.return_value = MagicMock(status_code=500, text="boom") + with pytest.raises(Exception, match="setup-token issuance failed"): + issue_setup_token("alice", headers) + + # --------------------------------------------------------------------------- # create_user_from_central_hub_user # --------------------------------------------------------------------------- -@patch("imaging_api.services.users.encrypt", return_value="encrypted_pwd") +@patch("imaging_api.services.users.encrypt", return_value="encrypted_setup") +@patch("imaging_api.services.users.issue_setup_token", return_value="/app/template/XDATScreen_UpdateUser.vm?a=al&s=se") @patch("imaging_api.services.users.create_user") @patch("imaging_api.services.users.to_create_imaging_user") -def test_create_user_from_central_hub_user(mock_to_create, mock_create, mock_encrypt, headers): +def test_create_user_from_central_hub_user(mock_to_create, mock_create, mock_issue, mock_encrypt, headers): mock_to_create.return_value = CreateUser( - username="alice", password="secret", firstName="Alice", + username="alice", password="secret", firstName="Alice", # pragma: allowlist secret lastName="A", email="alice@test.com", ) mock_create.return_value = User(**_SAMPLE_USER_DICT) @@ -317,8 +344,11 @@ def test_create_user_from_central_hub_user(mock_to_create, mock_create, mock_enc created_user, user_profile = create_user_from_central_hub_user(hub_user, headers) assert created_user.username == "alice" - assert created_user.encrypted_password == "encrypted_pwd" + # No password is returned — only the encrypted host-less set-password link (FLIP-PT-079). + assert created_user.encrypted_setup_path == "encrypted_setup" assert user_profile.username == "alice" + mock_issue.assert_called_once_with(user_profile.username, headers) + mock_encrypt.assert_called_once_with("/app/template/XDATScreen_UpdateUser.vm?a=al&s=se") # --------------------------------------------------------------------------- diff --git a/trust/xnat/docker-compose-stack.yml b/trust/xnat/docker-compose-stack.yml index 5b2d803d5..76dc2ba2f 100644 --- a/trust/xnat/docker-compose-stack.yml +++ b/trust/xnat/docker-compose-stack.yml @@ -43,6 +43,11 @@ services: - XNAT_HOME=${XNAT_HOME} - XNAT_ROOT=${XNAT_ROOT} - XNAT_EMAIL=${XNAT_EMAIL} + # Site branding for the enclave XNAT login page. TRUST_NAME comes from the trust kit + # (exported by trust/xnat/Makefile); configure-xnat.sh turns it into the site description + # ("Welcome to FLIP's XNAT at