Skip to content

[26.0] Fix oauth2 template validation#22253

Merged
dannon merged 3 commits intogalaxyproject:release_26.0from
davelopez:26.0_fix_oauth2_template_validation
Mar 26, 2026
Merged

[26.0] Fix oauth2 template validation#22253
dannon merged 3 commits intogalaxyproject:release_26.0from
davelopez:26.0_fix_oauth2_template_validation

Conversation

@davelopez
Copy link
Contributor

@davelopez davelopez commented Mar 25, 2026

Alternative to #22151 discussed in #22246 (comment)

Fixes #22041

The users now see:

image

And the logged error is:

Traceback (most recent call last):
  File "/home/dlopez/dev/gx-version/26.0/lib/galaxy/managers/file_source_instances.py", line 649, in template_server_configuration
    environment = prepare_environment_from_root(template.environment, self._app_vault, self._app_config)
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/dlopez/dev/gx-version/26.0/lib/galaxy/managers/_config_templates.py", line 293, in prepare_environment_from_root
    raise ConfigurationError(
galaxy.exceptions.ConfigurationError: Environment variable GALAXY_GOOGLE_DRIVE_APP_CLIENT_ID not found for environment entry oauth2_client_id and no default provided. This variable is required to be set in the environment for the template to function. Please set this variable in the environment or provide a default value in the template definition.

and for secrets, a similar exception:

galaxy.exceptions.ConfigurationError: Secret vault key file_source_secrets/google_drive/app_client_id not found for environment entry oauth2_client_id and no default provided. This secret is required to be set in the vault for the template to function. Please set this secret in the vault or provide a default value in the template definition.

How to test the changes?

  • I've included appropriate automated tests.
  • This is a refactoring of components with existing test coverage.
  • Instructions for manual testing are as follows:
    1. [add testing steps and prerequisites here if you didn't write automated tests covering all your changes]

License

  • I agree to license these and all my past contributions to the core galaxy codebase under the MIT license.

Fails fast when required secrets or environment variables are missing and no default is provided, instead of letting empty values reach OAuth setup.

Surfaces a clear configuration problem for administrators and a safe user-facing error, preventing downstream OAuth client pair validation failures.
Improves coverage around misconfigured file source templates and OAuth setup.

Ensures missing environment values and secrets fail with clear configuration errors, and missing OAuth credentials surface an actionable user-facing message for administrator follow-up.
@davelopez
Copy link
Contributor Author

@mvdbeek is this more in line with what you expect?

@mvdbeek
Copy link
Member

mvdbeek commented Mar 25, 2026

No, sorry. The environment variable is set, there's just nothing under the actual vault key.

Sorry, I saw Environment variable GALAXY_GOOGLE_DRIVE_APP_CLIENT_ID not found for environment entry oauth2_client_id first. It's closer

@mvdbeek
Copy link
Member

mvdbeek commented Mar 25, 2026

IOW this is a misleading message. I'm fine to call it wontfix, but we shouldn't say it's not in the environment variables when in fact it is.

Updates missing vault secrets and environment variables to surface as internal server errors instead of configuration errors.
oauth2_scope = None
if oauth2_configuration is not None:
environment = prepare_environment_from_root(template.environment, self._app_vault, self._app_config)
try:
Copy link
Member

@mvdbeek mvdbeek Mar 25, 2026

Choose a reason for hiding this comment

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

You could just let the exception bubble out, InternalServerError is a MessageException subclass, IIRC it's also logged

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If I let the exception bubble out, the message shown to the user will be:

image

Looks rather internal and confusing to the user, which is why I wrapped it.

Copy link
Member

@mvdbeek mvdbeek left a comment

Choose a reason for hiding this comment

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

Awesome, thank you so much, I think that results in clear error messages!

Copy link
Member

@dannon dannon left a comment

Choose a reason for hiding this comment

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

@davelopez Thank you for taking a look at this, looks great to me!

@dannon dannon merged commit ebe5606 into galaxyproject:release_26.0 Mar 26, 2026
53 of 56 checks passed
@github-project-automation github-project-automation bot moved this from Needs Review to Done in Galaxy Dev - weeklies Mar 26, 2026
@davelopez davelopez deleted the 26.0_fix_oauth2_template_validation branch March 26, 2026 09:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Development

Successfully merging this pull request may close these issues.

3 participants