Add new OIDC users to default group - #1105
Conversation
WalkthroughThe PR fixes OIDC user creation so that new users get added to the configured default IRIS group, matching existing LDAP behavior. The .env.model comment is corrected to state that just-in-time creation applies to both LDAP and OIDC authentication. ChangesOIDC default group assignment
Estimated code review effort: 1 (Trivial) | ~5 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.env.model:
- Line 52: Update the comment describing just-in-time LDAP/OIDC user creation to
replace “authentification” with “authentication,” without changing its meaning
or surrounding content.
In `@source/app/blueprints/pages/login/login_routes.py`:
- Around line 220-221: The get_group_by_name function can return None when the
configured group does not exist in the database, but the code immediately
dereferences initial_group.group_id without validation, causing an
AttributeError during OIDC just-in-time user creation. Add a None-check after
the get_group_by_name call to validate that initial_group is not None before
accessing its group_id attribute. If initial_group is None, return a clear
configuration error message indicating that the group specified in
IRIS_NEW_USERS_DEFAULT_GROUP does not exist in the database. Only proceed with
the add_user_to_group call once initial_group has been validated as non-None.
- Around line 220-221: Update the OIDC user-creation flow around create_user(),
get_group_by_name(), and add_user_to_group() so user persistence and
default-group membership share one transaction. Ensure any failure during group
lookup or assignment rolls back the newly created user and membership changes,
preventing a persisted user without the configured default group.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 72d66747-8186-4db3-a01e-f0f29c1fca56
📒 Files selected for processing (2)
.env.modelsource/app/blueprints/pages/login/login_routes.py
| #IRIS_ADM_EMAIL=admin@localhost | ||
| #IRIS_ADM_USERNAME=administrator | ||
| # requests the just-in-time creation of users with ldap authentification (see https://github.com/dfir-iris/iris-web/issues/203) | ||
| # requests the just-in-time creation of users with ldap / oidc authentification (see https://github.com/dfir-iris/iris-web/issues/203) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Correct the authentication spelling.
The changed comment still uses authentification. Replace it with authentication.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.env.model at line 52, Update the comment describing just-in-time LDAP/OIDC
user creation to replace “authentification” with “authentication,” without
changing its meaning or surrounding content.
| initial_group = get_group_by_name(app.config.get('IRIS_NEW_USERS_DEFAULT_GROUP')) | ||
| add_user_to_group(user.id, initial_group.group_id) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Validate the configured group before dereferencing it.
If IRIS_NEW_USERS_DEFAULT_GROUP does not match a database row, get_group_by_name returns None (source/app/datamgmt/manage/manage_groups_db.py:81-83). Line 221 then raises AttributeError, and OIDC just-in-time user creation fails. Resolve and validate initial_group before create_user, then return a clear configuration error when the group is missing.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@source/app/blueprints/pages/login/login_routes.py` around lines 220 - 221,
The get_group_by_name function can return None when the configured group does
not exist in the database, but the code immediately dereferences
initial_group.group_id without validation, causing an AttributeError during OIDC
just-in-time user creation. Add a None-check after the get_group_by_name call to
validate that initial_group is not None before accessing its group_id attribute.
If initial_group is None, return a clear configuration error message indicating
that the group specified in IRIS_NEW_USERS_DEFAULT_GROUP does not exist in the
database. Only proceed with the add_user_to_group call once initial_group has
been validated as non-None.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 20 'def create_user|def add_user_to_group|db\.session\.(commit|rollback)' source/appRepository: dfir-iris/iris-web
Length of output: 50375
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== target file outline/sections =="
wc -l source/app/blueprints/pages/login/login_routes.py source/app/datamgmt/manage/manage_users_db.py || true
sed -n '180,250p' source/app/blueprints/pages/login/login_routes.py
printf '\n== manage_users_db relevant sections ==\n'
sed -n '240,355p' source/app/datamgmt/manage/manage_users_db.py
printf '\n== precise function definitions/callers/usages in relevant datamgmt files ==\n'
rg -n -C 12 'def create_user|def add_user_to_group|get_group_by_name|create_user\s*\(|add_user_to_group\s*\(' source/app/datamgmt/manage source/app/blueprints/pages/login/login_routes.py
printf '\n== db transaction declarations in relevant files ==\n'
rg -n 'db\.session\.(commit|rollback)|before_request|atexit|`@app`\.(teardown|before)|after_request|sessionmaker|scoped_session|transaction' source/app/datamgmt/manage/manage_users_db.py source/app/blueprints/pages/login/login_routes.pyRepository: dfir-iris/iris-web
Length of output: 18844
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== create_user implementation =="
sed -n '673,730p' source/app/datamgmt/manage/manage_users_db.py
echo "== User.save definition candidates =="
rg -n -C 8 'class User|def save\(' source/app source -g '*.py' | head -n 120
echo "== OIDC login route full function =="
sed -n '130,230p' source/app/blueprints/pages/login/login_routes.py
echo "== search for session rollback handling in OIDC path =="
rg -n -C 6 'add_user_to_group|create_user\(|IRIS_NEW_USERS_DEFAULT_GROUP|db\.session\.rollback|rollback\(\)' source/app/blueprints/pages/login source/app/datamgmt -g '*.py'Repository: dfir-iris/iris-web
Length of output: 23104
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== User save implementation =="
sed -n '199,250p' source/app/models/authorization.py
echo "== ac_auto_update_user_effective_access implementation =="
rg -n -C 12 'def ac_auto_update_user_effective_access|_auto_update_user_effective_access' source/app/datamgmt -g '*.py'Repository: dfir-iris/iris-web
Length of output: 24376
Make OIDC user creation and default-group assignment atomic.
create_user() commits the user after User.save(), and add_user_to_group() commits membership separately. If default-group assignment fails, the OIDC user remains persisted without the configured membership. Wrap the creation sequence in one transaction, or roll back create_user() changes if assignment fails.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@source/app/blueprints/pages/login/login_routes.py` around lines 220 - 221,
Update the OIDC user-creation flow around create_user(), get_group_by_name(),
and add_user_to_group() so user persistence and default-group membership share
one transaction. Ensure any failure during group lookup or assignment rolls back
the newly created user and membership changes, preventing a persisted user
without the configured default group.
1 participant
Closes #1104
Summary by CodeRabbit
New Features
Documentation