fix(auth): resolve DetachedInstanceError on User.identity during registration#686
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a7a5529445
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| "registration_source": registration_source, | ||
| "is_active": is_active or identity.is_platform_admin, | ||
| }) | ||
| user.identity = identity |
There was a problem hiding this comment.
When /register/init reuses an Identity whose password_hash is NULL, the check in backend/app/api/auth.py:179 skips password verification entirely; channel/proactive users are created this way with password_hash=None in backend/app/services/channel_user_service.py. This new eager assignment now lets UserOut serialize successfully instead of hitting the DetachedInstanceError, so an attacker who knows such an email can submit any password and receive an access token for the created/reused user. Add an explicit reject/set-password flow before creating or returning a user for passwordless identities.
Useful? React with 👍 / 👎.
| environment: | ||
| VITE_API_URL: http://localhost:8000 | ||
| API_UPSTREAM: backend:8000 |
There was a problem hiding this comment.
The CI compose file starts the frontend, but this environment block omits MINIO_UPSTREAM even though frontend/nginx.conf.template:57 and :60 reference ${MINIO_UPSTREAM} for the /minio/ proxy. In CI, where that variable is not otherwise provided, the nginx template is rendered/parsed with an invalid upstream/Host value (unlike the main compose files, which set ${MINIO_UPSTREAM:-minio:9000}), so the frontend container exits while the deploy test only waits on the backend.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f4df71a04b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| branch: | ||
| - main | ||
| - release | ||
| - ci/test-drone # 临时添加，测试完成后删除 |
There was a problem hiding this comment.
When this YAML is merged, pushes to a ci/test-drone branch will run the full pipeline, including save-images and scp-images to /opt/server/clawith-ci/, even though publishing should be limited to main/release. Since the inline note says this was temporary for testing, drop this branch before merging so test branches cannot overwrite CI deployment artifacts.
Useful? React with 👍 / 👎.
1 participant
Summary
This PR fixes a
DetachedInstanceErrorthat was raised during user registration flows (both normal/register/initand SSO/register/sso).Cause of the bug
registration_service.create_user_with_identity(usinguser_dao.create), it only writes theidentity_idforeign key. Theidentityrelationship attribute is not loaded/populated in Python memory.Userobject becomes detached.UserOutreadsuser.emailanduser.username(which are association proxies delegating touser.identity). Sinceuser.identityis not loaded in memory, SQLAlchemy tries to lazy-load it on a detached instance, resulting in:Fix
user.identity = identityin memory when:registration_service.create_user_with_identityauth.py'sregister_initflowThis populates the relationship in Python memory and avoids database lazy loading on serialization.