Skip to content

[PM-37864] feat: Hide create account button based on server disableUserRegistration config#2865

Open
fedemkr wants to merge 7 commits into
mainfrom
PM-37864/hide-create-account-fedramp
Open

[PM-37864] feat: Hide create account button based on server disableUserRegistration config#2865
fedemkr wants to merge 7 commits into
mainfrom
PM-37864/hide-create-account-fedramp

Conversation

@fedemkr

@fedemkr fedemkr commented Jul 7, 2026

Copy link
Copy Markdown
Member

🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-37864

📔 Objective

Adds a settings.disableUserRegistration field to ConfigResponseModel and the ServerConfig domain model, populated from the server's /config response.

The landing processor subscribes to configPublisher in init and reactively updates LandingState.isCreateAccountButtonHidden whenever a pre-auth config is emitted. The "New around here? Create account" row in LandingView is removed from the view hierarchy entirely when the flag is true.

fedemkr added 5 commits July 6, 2026 11:13
… selector

Adds a `.gov` RegionType case backed by bitwarden-gov.com default URLs so
users can select the new government cloud environment from the region
selector, ahead of the broader FedRAMP support work tracked in PM-35110.
# Conflicts:
#	BitwardenKit/Core/Platform/Models/Domain/EnvironmentURLData.swift
#	BitwardenKit/Core/Platform/Models/Domain/EnvironmentURLs.swift
#	BitwardenShared/Core/Platform/Models/Enum/RegionType.swift
#	BitwardenShared/Core/Platform/Models/Enum/RegionTypeTests.swift
#	BitwardenShared/Core/Platform/Services/EnvironmentService.swift
#	BitwardenShared/UI/Auth/Utilities/RegionHelper.swift
…erRegistration config

Adds `settings.disableUserRegistration` to `ConfigResponseModel` and `ServerConfig`.
The landing processor subscribes to `configPublisher` in `init` and reactively hides
the "New around here? Create account" row whenever the server signals that user
registration is disabled.
@fedemkr fedemkr added the ai-review Request a Claude code review label Jul 7, 2026
@github-actions github-actions Bot added app:password-manager Bitwarden Password Manager app context app:authenticator Bitwarden Authenticator app context t:feature labels Jul 7, 2026
@fedemkr fedemkr marked this pull request as ready for review July 7, 2026 21:09
@fedemkr fedemkr requested review from a team and matt-livefront as code owners July 7, 2026 21:09
@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

Reviewed the addition of settings.disableUserRegistration to ConfigResponseModel/ServerConfig, the reactive hiding of the "Create account" row in LandingView via a configPublisher subscription in LandingProcessor, and the new .gov (FedRAMP) RegionType with its environment URLs. The new region cases are handled exhaustively across all switch statements and are well covered by tests. One non-blocking lifecycle concern noted on the processor subscription.

Code Review Details
  • ⚠️ : Task in init strongly captures self over a never-completing publisher, leaking the screen-scoped processor
    • BitwardenShared/UI/Auth/Landing/LandingProcessor.swift:62

@codecov

codecov Bot commented Jul 7, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 28.57143% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 40.57%. Comparing base (85f3884) to head (a94616c).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...Kit/Core/Platform/Models/Domain/ServerConfig.swift 0.00% 7 Missing ⚠️
...rm/Services/API/Response/ConfigResponseModel.swift 0.00% 4 Missing ⚠️
...re/Platform/Models/Domain/EnvironmentURLData.swift 71.42% 2 Missing ⚠️
...rdenKit/Core/Platform/Models/Enum/RegionType.swift 0.00% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main    #2865       +/-   ##
===========================================
- Coverage   81.27%   40.57%   -40.71%     
===========================================
  Files        1027      357      -670     
  Lines       66109    16675    -49434     
===========================================
- Hits        53733     6766    -46967     
+ Misses      12376     9909     -2467     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment on lines +62 to +68
Task {
for await metaConfig in await services.configService.configPublisher() {
guard metaConfig?.isPreAuth == true else { continue }
self.state.isCreateAccountButtonHidden = metaConfig?.serverConfig?
.settings?.disableUserRegistration == true
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ IMPORTANT: This Task strongly captures self, leaking the processor.

Details and fix

configPublisher() is backed by a Never-failing subject that never completes, so this for await loop runs indefinitely and the Task retains self (the LandingProcessor) for the lifetime of the app. When the landing screen is dismissed, the processor cannot deallocate.

Unlike ClientService / ServerCommunicationConfigClientSingleton (which use this pattern but are app-lifetime singletons where the retain is harmless), a StateProcessor is screen-scoped and expected to deallocate. Other processors that hold long-lived tasks use [weak self] (e.g. VaultListProcessor, AppProcessor).

Task { [weak self] in
    guard let configPublisher = await self?.services.configService.configPublisher() else { return }
    for await metaConfig in configPublisher {
        guard metaConfig?.isPreAuth == true else { continue }
        self?.state.isCreateAccountButtonHidden = metaConfig?.serverConfig?
            .settings?.disableUserRegistration == true
    }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-review Request a Claude code review app:authenticator Bitwarden Authenticator app context app:password-manager Bitwarden Password Manager app context t:feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant