Skip to content

Enable update_user_role property for Docker Image #350

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
tjferguson wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Enable update_user_role property for Docker Image #350

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e3bbd69
Add the redirect_uri configuration to the allowed env variables
tjferguson Dec 11, 2025
f370b78
Merge branch 'MISP:master' into master
tjferguson Dec 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion core/files/configure_misp.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,8 @@ set_up_oidc() {
\"default_org\": \"${OIDC_DEFAULT_ORG}\",
\"mixedAuth\": ${OIDC_MIXEDAUTH},
\"authentication_method\": \"${OIDC_AUTH_METHOD}\",
\"redirect_uri\": \"${OIDC_REDIRECT_URI}\"
\"redirect_uri\": \"${OIDC_REDIRECT_URI}\",
+ \"update_user_role\": ${OIDC_UPDATE_USER_ROLE}
Copy link
Copy Markdown
Collaborator

@ostefano ostefano Dec 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens when this is empty? Is this valid?

Copy link
Copy Markdown
Author

@tjferguson tjferguson Dec 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is referenced in:
https://github.com/MISP/MISP/blob/2.5/app/Plugin/OidcAuth/Lib/Oidc.php#L109
https://github.com/MISP/MISP/blob/2.5/app/Plugin/OidcAuth/Lib/Oidc.php#L240

The bit that matters for this is:

$this->getConfig('update_user_role', true)

This uses the getConfig function from:
https://github.com/MISP/MISP/blob/2.5/app/Plugin/OidcAuth/Lib/Oidc.php#L541

If the value is not in the configuration file at all, then getConfig call below would default the value to true (which is what the container does today).

In the case where the env variable is empty coming from the container runtime, you are 100% right; this would cause the JSON being written to be invalid. We should probably use ${OIDC_UPDATE_USER_ROLE:=default_role_value} to account for this case, so this isn't a breaking change for existing configs.

Copy link
Copy Markdown
Collaborator

@ostefano ostefano Dec 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair, but does /var/www/MISP/tests/modify_config.php modify succeed when that is the case?

Copy link
Copy Markdown
Author

@tjferguson tjferguson Dec 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it does, if I'm reading the modify_config.php file correctly; otherwise the upstream project would have issues with the upgrades after the new property was added.

Using:
${OIDC_UPDATE_USER_ROLE:=true}

Should allow any potential inputs to pass as they currently do. There is a failure case in the original submission for an invalid json file if no value was set.

I'll give the following a test when I get back to a computer:

  1. input value set to true/false
  2. no input value supplied for OIDC_UPDATE_USER_ROLE
  3. no input value specified, but a value already set in an existing container (e.g. via a configmap in k8s)

Are there any other scenarios this needs to be tested under?

Copy link
Copy Markdown
Collaborator

@ostefano ostefano Dec 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we are sure that the default value is true, we can cut to the chase by creating defaults in the main entrypoint file (entrypoint.sh) so we are sure that the var is always initialized to a default value.

Also, we should update docker-compose.yml and template.env to document it.

}
}" > /dev/null

Expand Down