Introduce vendor namespace support - #7
Conversation
Adds a `vendor` option to ThunderIDConfig so adopters can white-label the SDK's runtime identifiers (Keychain service name, i18n locale storage key) instead of being pinned to the `thunderid` brand prefix. Related: thunder-id/thunderid#3896
|
Warning Ignoring CodeRabbit configuration file changes. For security, only the configuration from the base branch is applied for open source repositories. 📝 WalkthroughWalkthroughChangesVendor-scoped identifiers
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant ThunderIDConfig
participant ThunderIDClient
participant KeychainStorageAdapter
participant ThunderIDViewModifier
ThunderIDConfig->>ThunderIDClient: configured vendor
ThunderIDClient->>KeychainStorageAdapter: service "dev.<vendor>.sdk"
ThunderIDConfig->>ThunderIDViewModifier: configured vendor
ThunderIDViewModifier->>ThunderIDViewModifier: resolve "<vendor>_locale"
🚥 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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
Sources/ThunderID/ThunderIDClient.swift (2)
65-65: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
reInitializesilently resetsvendorwhenbaseUrlis provided.Line 65 creates a new
ThunderIDConfigwith onlybaseUrlandclientId, causingvendor(and all other fields) to fall back to defaults. After re-initialization, the SDK reads tokens from"dev.thunderid.sdk"instead of the custom vendor's keychain namespace, silently losing access to existing tokens for white-label customers.This is a pre-existing bug that loses all config fields, but the addition of
vendormakes it a data-integrity hazard for the white-labeling use case this PR introduces.🛡️ Proposed fix: preserve vendor in reInitialize
- if let baseUrl { current = ThunderIDConfig(baseUrl: baseUrl, clientId: current.clientId) } + if let baseUrl { + current = ThunderIDConfig( + baseUrl: baseUrl, + clientId: current.clientId, + vendor: current.vendor + ) + }Note: ideally all relevant config fields should be preserved, not just
vendor.🤖 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 `@Sources/ThunderID/ThunderIDClient.swift` at line 65, Update reInitialize to preserve the existing ThunderIDConfig fields when replacing baseUrl, especially vendor, rather than constructing a config with only baseUrl and clientId. Use the configuration update/copy mechanism in reInitialize so all relevant values, including the custom keychain namespace, remain unchanged.
331-338: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAdd validation for empty
vendorinvalidateConfig.An empty
vendorproduces malformed identifiers: keychain service"dev..sdk"and locale storage key"_locale". A simple guard prevents this edge case.🛡️ Proposed fix
private func validateConfig(_ config: ThunderIDConfig) throws { guard !config.baseUrl.isEmpty else { throw ThunderIDError(code: .invalidConfiguration, message: "baseUrl is required") } guard config.baseUrl.hasPrefix("https://") else { throw ThunderIDError(code: .invalidConfiguration, message: "baseUrl must use HTTPS") } + guard !config.vendor.isEmpty else { + throw ThunderIDError(code: .invalidConfiguration, message: "vendor is required") + } }🤖 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 `@Sources/ThunderID/ThunderIDClient.swift` around lines 331 - 338, Add a guard in validateConfig(_:) to reject an empty config.vendor value, throwing ThunderIDError with code .invalidConfiguration and a clear message such as "vendor is required", alongside the existing baseUrl validation.
🧹 Nitpick comments (1)
Sources/ThunderID/VendorConstants.swift (1)
22-26: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider extracting vendor-derived identifier helpers.
Vendor-derived identifier construction is now repeated across
ThunderIDClient.swift("dev.\(config.vendor).sdk") andThunderIDViewModifier.swift("\(config.vendor)_locale"). Adding static helpers here would centralize the format strings, prevent divergence, and allowKeychainStorageAdapter's default to resolve viaVendorConstantsinstead of hardcoding"dev.thunderid.sdk".As per coding guidelines: "Extract shared helpers when vendor default-resolution logic is repeated."
♻️ Proposed helpers
public enum VendorConstants { /// The prefix used for vendor-specific storage identifiers (e.g. Keychain service name), or other /// runtime keys/names. public static let vendorPrefix: String = "thunderid" + + /// Default Keychain service identifier for a given vendor. + public static func keychainService(for vendor: String) -> String { + "dev.\(vendor).sdk" + } + + /// Default locale storage key for a given vendor. + public static func localeStorageKey(for vendor: String) -> String { + "\(vendor)_locale" + } }🤖 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 `@Sources/ThunderID/VendorConstants.swift` around lines 22 - 26, Centralize vendor-derived identifier formatting in VendorConstants by adding static helpers for the SDK keychain service identifier and locale key, then replace the duplicated "dev.\(config.vendor).sdk" and "\(config.vendor)_locale" constructions in ThunderIDClient and ThunderIDViewModifier with those helpers. Update KeychainStorageAdapter’s default service identifier to use the helper instead of hardcoding "dev.thunderid.sdk".Source: Coding guidelines
🤖 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 `@Sources/ThunderIDSwiftUI/environment/ThunderIDViewModifier.swift`:
- Around line 48-50: Preserve existing locale preferences when the default
storage key changes. Update ThunderIDI18n.init to use the new key first, fall
back to the legacy "thunder_locale" value when no new-key value exists, and
migrate that value to the new key; ensure thunderIDProvider continues passing
the vendor-based key through ThunderIDProviderModifier.
---
Outside diff comments:
In `@Sources/ThunderID/ThunderIDClient.swift`:
- Line 65: Update reInitialize to preserve the existing ThunderIDConfig fields
when replacing baseUrl, especially vendor, rather than constructing a config
with only baseUrl and clientId. Use the configuration update/copy mechanism in
reInitialize so all relevant values, including the custom keychain namespace,
remain unchanged.
- Around line 331-338: Add a guard in validateConfig(_:) to reject an empty
config.vendor value, throwing ThunderIDError with code .invalidConfiguration and
a clear message such as "vendor is required", alongside the existing baseUrl
validation.
---
Nitpick comments:
In `@Sources/ThunderID/VendorConstants.swift`:
- Around line 22-26: Centralize vendor-derived identifier formatting in
VendorConstants by adding static helpers for the SDK keychain service identifier
and locale key, then replace the duplicated "dev.\(config.vendor).sdk" and
"\(config.vendor)_locale" constructions in ThunderIDClient and
ThunderIDViewModifier with those helpers. Update KeychainStorageAdapter’s
default service identifier to use the helper instead of hardcoding
"dev.thunderid.sdk".
🪄 Autofix (Beta)
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: eb06a02e-b3ef-4c71-b9a5-5c61c9356812
📒 Files selected for processing (6)
.coderabbit.yamlAGENTS.mdSources/ThunderID/ThunderIDClient.swiftSources/ThunderID/ThunderIDConfig.swiftSources/ThunderID/VendorConstants.swiftSources/ThunderIDSwiftUI/environment/ThunderIDViewModifier.swift
| func thunderIDProvider(config: ThunderIDConfig, i18n: ThunderIDI18n? = nil) -> some View { | ||
| let resolvedI18n = i18n ?? ThunderIDI18n(storageKey: "\(config.vendor)_locale") | ||
| return modifier(ThunderIDProviderModifier(config: config, i18n: resolvedI18n)) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Default i18n storage key changes from "thunder_locale" to "thunderid_locale", silently losing existing locale preferences.
The previous default ThunderIDI18n() used storageKey: "thunder_locale". The new default ThunderIDI18n(storageKey: "\(config.vendor)_locale") produces "thunderid_locale" for the default vendor. Existing users upgrading will have their stored locale preference orphaned under the old key, silently reverting to the fallback locale.
Consider a migration fallback in ThunderIDI18n.init — if the new key has no stored value, read from the old key and migrate.
🛡️ Proposed migration fallback in ThunderIDI18n.init
public init(
bundles: [String: [String: String]] = [:],
language: String? = nil,
fallbackLanguage: String = "en-US",
storageKey: String = "thunder_locale"
) {
self.bundles = bundles
self.fallbackLocale = fallbackLanguage
self.storageKey = storageKey
- let stored = UserDefaults.standard.string(forKey: storageKey)
+ let defaults = UserDefaults.standard
+ var stored = defaults.string(forKey: storageKey)
+ if stored == nil, storageKey != "thunder_locale" {
+ stored = defaults.string(forKey: "thunder_locale")
+ if let stored { defaults.set(stored, forKey: storageKey) }
+ }
self.activeLocale = language ?? stored ?? fallbackLanguage
}🤖 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 `@Sources/ThunderIDSwiftUI/environment/ThunderIDViewModifier.swift` around
lines 48 - 50, Preserve existing locale preferences when the default storage key
changes. Update ThunderIDI18n.init to use the new key first, fall back to the
legacy "thunder_locale" value when no new-key value exists, and migrate that
value to the new key; ensure thunderIDProvider continues passing the
vendor-based key through ThunderIDProviderModifier.
2 participants
Purpose
ThunderIDis hardcoded in some SDK components, making it hard for adopters to white-label the SDK. Mirrors the approach landed in thunder-id/javascript-sdks#24.Approach
vendoroption toThunderIDConfig(defaults toVendorConstants.vendorPrefix,"thunderid").ThunderIDClientnow derives the default Keychain service name fromconfig.vendorinstead of a hardcoded string.thunderIDProvider(config:i18n:)now resolves the i18n locale storage key fromconfig.vendorwhen no explicitThunderIDI18nis supplied.AGENTS.mdso future changes keep runtime keys/names vendor-aware.Related Issues
Checklist
swift buildpasses.swiftlint lint --strictpasses with 0 violations.Summary by CodeRabbit