Skip to content

feat(server): Add config option to disable admin registration#24539

Closed
provokateurin wants to merge 1 commit intoimmich-app:mainfrom
provokateurin:feat/server/config-option-disable-admin-registration
Closed

feat(server): Add config option to disable admin registration#24539
provokateurin wants to merge 1 commit intoimmich-app:mainfrom
provokateurin:feat/server/config-option-disable-admin-registration

Conversation

@provokateurin
Copy link

@provokateurin provokateurin commented Dec 12, 2025

Description

Since it is possible to create Immich admin users via OIDC, registering an admin user first is no longer a strict requirement. Therefore an option (default disabled) is needed to skip this step.

Fixes #18847

How Has This Been Tested?

  • Setup a fresh instance with OIDC config. Open the web page, get redirect to the OIDC provider instead of having to register an admin user first.

Checklist:

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation if applicable
  • I have no unrelated changes in the PR.
  • I have confirmed that any new dependencies are strictly necessary.
  • I have written tests for new code (if applicable)
  • I have followed naming conventions/patterns in the surrounding code
  • All code in src/services/ uses repositories implementations for database calls, filesystem operations, etc.
  • All code in src/repositories/ is pretty basic/simple and does not have any immich specific logic (that belongs in src/services/)

Please describe to which degree, if any, an LLM was used in creating this pull request.

None, all written by hand.

@immich-push-o-matic immich-push-o-matic bot added documentation Improvements or additions to documentation 🗄️server labels Dec 12, 2025
Copy link
Member

@danieldietzler danieldietzler left a comment

Choose a reason for hiding this comment

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

Thanks!

Copy link
Member

@jrasm91 jrasm91 left a comment

Choose a reason for hiding this comment

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

You need to expose this in the API, by updating SystemConfigOAuthDto. You should also add this option as a switch/toggle in the web ui.

@provokateurin
Copy link
Author

Ok, I'll have a look!

@provokateurin
Copy link
Author

I added a switch, but while trying to confirm that it works i realized that it is pretty useless. You can only see the settings page if you have already registered an admin user and are logged in as this user. So this config option does not make sense if you don't use the config.json file. Also disabling after already having created an admin user will have no effect.

@jrasm91 what do you think?

@provokateurin provokateurin force-pushed the feat/server/config-option-disable-admin-registration branch from 3e94bdb to d318d64 Compare December 15, 2025 18:38
@jrasm91
Copy link
Member

jrasm91 commented Dec 15, 2025

Oh, true. Probably we don't need the switch in the UI, since it's useless in this case. I think we should probably still add it to the dto though, since that is used for validation. I'm actually surprised that the functionality would work without validation. How is it loading the value from the config file if wasn't added to the dto?

@provokateurin
Copy link
Author

Isn't the Dto only used for API stuff? As far as I could tell it's not really used in this case, as the config value is only used on the server.

@provokateurin provokateurin force-pushed the feat/server/config-option-disable-admin-registration branch from d318d64 to a03fd3f Compare December 15, 2025 19:39
@provokateurin provokateurin force-pushed the feat/server/config-option-disable-admin-registration branch from a03fd3f to 60edd06 Compare December 15, 2025 19:56
@jrasm91
Copy link
Member

jrasm91 commented Dec 16, 2025

It uses it for validation, even when it is loaded from a file. You should be able to see this easily by using a string instead of a boolean for the new property and it will load the file just fine. After you add the property and @ValidateBoolean() to the dto, it will start throwing an error.

https://github.com/immich-app/immich/blob/main/server/src/utils/config.ts#L105

@jrasm91
Copy link
Member

jrasm91 commented Dec 16, 2025

After some thought, I've realized a few things:

  • oauth.disableAdminRegistration makes me think this disallows creating admin accounts through OAuth, which this does not prevent.
  • We just want to skip and disable the admin sign up form, which uses a password
  • passwordLogin.enabled already exists and could just be used instead

What do you think about leaving all the config as is, and just extending the auth/admin-sign-up endpoint to not work if password login is disabled? Additionally, we can also set isInitalized if there is an admin or password login is disabled.

@jrasm91
Copy link
Member

jrasm91 commented Dec 16, 2025

OK, apparently there are other use cases for wanting to disable the sign up endpoint, regardless of password or oauth setup. Do you mind putting here instead?

firstTimeSetup: {
  enabled: boolean;
}

@jrasm91
Copy link
Member

jrasm91 commented Dec 16, 2025

Sorry for all the noise. I decided to implement this as an ENV instead of a config, and went ahead and did it myself in #24628.

@jrasm91 jrasm91 closed this Dec 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog:enhancement documentation Improvements or additions to documentation 🗄️server

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants