Entitlements and Hard Check #35
Conversation
- Add .build/ directory to .gitignore - Add Package.resolved to .gitignore - Remove SPM artifacts from Git tracking
WalkthroughAdds API client classes and Codable response models for permissions, roles, and feature flags; introduces AnyCodable, entitlement/flag domain types, ClaimsService/EntitlementsService/FeatureFlagsService and Auth extensions; expands AuthError; updates tests, Xcode project files, .gitignore, workspace, and clears Package.resolved. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor App as App Code
participant Auth as Auth
participant Claims as ClaimsService
participant Ent as EntitlementsService
participant FF as FeatureFlagsService
participant API as Kinde API
App->>Auth: configure()
note right of Auth: exposes claims, entitlements, featureFlags
App->>Ent: fetchEntitlements(forceApi?)
Ent->>Auth: isAuthenticated? / getTokenResponse()
alt Not authenticated
Ent-->>App: throws NotAuthenticated
else Authenticated
Ent->>API: GET /account_api/v1/entitlements (with token)
API-->>Ent: JSON (possibly paginated)
Ent-->>App: Entitlements + metadata
end
App->>FF: isFeatureEnabled(key, default, forceApi?)
FF->>Auth: read claims or call API (forceApi)
FF-->>App: Bool or default
App->>Claims: getClaim(key)
Claims->>Auth: read token claims
Claims-->>App: Claim(name, AnyCodable value)
sequenceDiagram
autonumber
actor App as App Code
participant Ent as EntitlementsService
participant API as Kinde API
App->>Ent: getBooleanEntitlement(key, default)
Ent->>Ent: lookup in claims or call API (forceApi?)
alt Key present & convertible
Ent-->>App: true/false
else Missing or not-convertible
Ent-->>App: default or throws depending on method
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Files/areas needing extra attention:
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🔇 Additional comments (2)
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: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/KindeSDK/Auth/Auth.swift (1)
161-207: Fix claim unwrapping (permissions/org helpers now always return nil).We now wrap claim payloads in
AnyCodable, but these helpers still cast the wrapper itself (claim.value as? ...). That cast always fails, sogetPermissions(),getPermission(_:),getOrganization(), andgetUserOrganizations()can no longer return real data. Please unwrap via.value(the underlyingAny) before casting. Suggested patch:- if let permissionsClaim = getClaim(forKey: ClaimKey.permissions.rawValue), - let permissionsArray = permissionsClaim.value as? [String], - let orgCodeClaim = getClaim(forKey: ClaimKey.organisationCode.rawValue), - let orgCode = orgCodeClaim.value as? String { + if let permissionsClaim = getClaim(forKey: ClaimKey.permissions.rawValue), + let permissionsArray = permissionsClaim.value.value as? [String], + let orgCodeClaim = getClaim(forKey: ClaimKey.organisationCode.rawValue), + let orgCode = orgCodeClaim.value.value as? String {Apply the same fix in the three sibling helpers above. Without this, every call now silently fails.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
.gitignore(1 hunks)KindeSDK.xcodeproj/project.pbxproj(7 hunks)KindeSDK.xcodeproj/project.pbxproj.backup(1 hunks)Package.resolved(0 hunks)Sources/KindeSDK/Auth/Auth.swift(5 hunks)Sources/KindeSDK/Auth/AuthError.swift(2 hunks)Tests/AuthSpec.swift(2 hunks)Tests/EntitlementsSpec.swift(1 hunks)
💤 Files with no reviewable changes (1)
- Package.resolved
🧰 Additional context used
🧬 Code graph analysis (3)
Tests/EntitlementsSpec.swift (3)
Tests/AuthSpec.swift (1)
spec(7-88)Sources/KindeSDK/APIs.swift (1)
configure(120-153)Sources/KindeSDK/Auth/Auth.swift (15)
getEntitlements(941-965)getEntitlement(970-973)getBooleanEntitlement(1120-1130)getStringEntitlement(1137-1147)getNumericEntitlement(1154-1164)performHardCheck(1172-1179)getFeatureFlags(1194-1218)isFeatureEnabled(1233-1249)getFeatureFlag(1223-1226)getClaim(135-146)getClaim(913-919)fetchEntitlements(990-1040)fetchEntitlement(1045-1081)getAllEntitlements(1086-1097)getEntitlementsDictionary(1102-1111)
Tests/AuthSpec.swift (2)
Sources/KindeSDK/Auth/Auth.swift (9)
isAuthenticated(103-114)getUserDetails(116-132)getClaim(135-146)getClaim(913-919)getPermissions(161-173)getOrganization(189-196)getUserOrganizations(198-207)getFlag(635-637)logout(342-346)Sources/KindeSDK/APIs.swift (2)
configure(120-153)logout(103-111)
Sources/KindeSDK/Auth/Auth.swift (2)
Tests/DefaultLoggerSpec.swift (1)
error(16-18)Sources/KindeSDK/Auth/DefaultLogger.swift (1)
error(23-25)
🪛 SwiftLint (0.57.0)
Sources/KindeSDK/Auth/Auth.swift
[Warning] 1088-1088: Initializing an optional variable with nil is redundant
(redundant_optional_initialization)
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/KindeSDK/Auth/Auth.swift (2)
161-205: UnwrapAnyCodablebefore casting.
Claim.valueis now anAnyCodable, but these helpers still cast it directly to[String]/String. The casts always fail, so permissions, org, and user-org lookups now returnnil. Extract the underlying value via.valuefirst (same applies to other helpers in this file that read claim values).- if let permissionsClaim = getClaim(forKey: ClaimKey.permissions.rawValue), - let permissionsArray = permissionsClaim.value as? [String], - let orgCodeClaim = getClaim(forKey: ClaimKey.organisationCode.rawValue), - let orgCode = orgCodeClaim.value as? String { + if let permissionsClaim = getClaim(forKey: ClaimKey.permissions.rawValue), + let permissionsArray = permissionsClaim.value.value as? [String], + let orgCodeClaim = getClaim(forKey: ClaimKey.organisationCode.rawValue), + let orgCode = orgCodeClaim.value.value as? String { @@ - if let permissionsClaim = getClaim(forKey: ClaimKey.permissions.rawValue), - let permissionsArray = permissionsClaim.value as? [String], - let orgCodeClaim = getClaim(forKey: ClaimKey.organisationCode.rawValue), - let orgCode = orgCodeClaim.value as? String { + if let permissionsClaim = getClaim(forKey: ClaimKey.permissions.rawValue), + let permissionsArray = permissionsClaim.value.value as? [String], + let orgCodeClaim = getClaim(forKey: ClaimKey.organisationCode.rawValue), + let orgCode = orgCodeClaim.value.value as? String { @@ - if let orgCodeClaim = getClaim(forKey: ClaimKey.organisationCode.rawValue), - let orgCode = orgCodeClaim.value as? String { + if let orgCodeClaim = getClaim(forKey: ClaimKey.organisationCode.rawValue), + let orgCode = orgCodeClaim.value.value as? String { @@ - if let userOrgsClaim = getClaim(forKey: ClaimKey.organisationCodes.rawValue, - token: .idToken), - let userOrgs = userOrgsClaim.value as? [String] { + if let userOrgsClaim = getClaim(forKey: ClaimKey.organisationCodes.rawValue, + token: .idToken), + let userOrgs = userOrgsClaim.value.value as? [String] {Apply the same
.value.valueadjustment wherever a claim is consumed (e.g. feature-flag parsing below) to restore previous behaviour.
685-688: Also fix feature-flag claim casting.
featureFlagsClaim.valueis anAnyCodable; the current cast to[String: Any]will always fail, so every flag lookup now throwsFlagError.unknownError. Unwrap the payload first.- guard let featureFlags = featureFlagsClaim.value as? [String : Any] else { + guard let featureFlags = featureFlagsClaim.value.value as? [String : Any] else {
♻️ Duplicate comments (1)
Tests/EntitlementsSpec.swift (1)
292-325: Convert this spec toAsyncSpecbefore usingawait.These examples call
await expect { try await … }, but the spec still subclassesQuickSpec, so the compiler rejects the async calls (same problem noted previously). Switch toAsyncSpecand makespec()async—or drop theawaitexpectations.-class EntitlementsSpec: QuickSpec { - override class func spec() { +class EntitlementsSpec: AsyncSpec { + override class func spec() async {Also update any
beforeEach/itbodies toawaitas needed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
KindeSDK.xcodeproj/project.pbxproj(3 hunks)Sources/KindeSDK/Auth/Auth.swift(5 hunks)Tests/EntitlementsSpec.swift(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
Sources/KindeSDK/Auth/Auth.swift (3)
Sources/KindeSDK/Utils/CodableHelper.swift (2)
decode(42-44)encode(46-48)Sources/KindeSDK/Utils/String+JWTTokenDecode.swift (1)
decode(17-48)Sources/KindeSDK/Auth/DefaultLogger.swift (1)
error(23-25)
Tests/EntitlementsSpec.swift (3)
Tests/AuthSpec.swift (1)
spec(7-88)Sources/KindeSDK/APIs.swift (1)
configure(120-153)Sources/KindeSDK/Auth/Auth.swift (15)
getEntitlements(951-975)getEntitlement(980-983)getBooleanEntitlement(1130-1140)getStringEntitlement(1147-1157)getNumericEntitlement(1164-1174)performHardCheck(1182-1189)getFeatureFlags(1204-1228)isFeatureEnabled(1243-1259)getFeatureFlag(1233-1236)getClaim(135-146)getClaim(927-929)fetchEntitlements(1000-1050)fetchEntitlement(1055-1091)getAllEntitlements(1096-1107)getEntitlementsDictionary(1112-1121)
🪛 SwiftLint (0.57.0)
Sources/KindeSDK/Auth/Auth.swift
[Warning] 1098-1098: Initializing an optional variable with nil is redundant
(redundant_optional_initialization)
Tests/EntitlementsSpec.swift
[Warning] 41-41: TODOs should be resolved (Add proper test fixtures with ...)
(todo)
[Warning] 263-263: TODOs should be resolved (Add proper test fixtures with ...)
(todo)
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
victoreronmosele
left a comment
victoreronmosele
left a comment
There was a problem hiding this comment.
It appears there are some missing methods being referenced in the tests.
There's also a missing ); in the project file.
victoreronmosele
left a comment
victoreronmosele
left a comment
There was a problem hiding this comment.
It appears some files are not updated as some methods are missing making the tests not compile.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
Sources/KindeSDK/APIs/PermissionsAPI.swift (1)
10-46: Same refactoring opportunities as RolesAPI.PermissionsAPI has the same structure and SwiftLint warnings as RolesAPI. The recommendations from RolesAPI.swift apply here as well:
- Consider using a caseless enum instead of
final public class- Extract common request builder logic to reduce duplication
Based on static analysis hints from SwiftLint.
Sources/KindeSDK/APIs/FeatureFlagsAPI.swift (1)
10-46: Same refactoring opportunities as RolesAPI and PermissionsAPI.FeatureFlagsAPI follows the same pattern as the other API clients. Apply the same refactoring recommendations mentioned for RolesAPI.swift.
Based on static analysis hints from SwiftLint.
Tests/EntitlementsSpec.swift (1)
319-355: Async expectations requireAsyncSpecorwaitUntil.The
await expect { try await ... }pattern inside a synchronousQuickSpecwill not compile. Swift requires the enclosing context to be async to useawait.Convert to
AsyncSpecor usewaitUntilblocks:-class EntitlementsSpec: QuickSpec { +class EntitlementsSpec: AsyncSpec {Or wrap each async test in
waitUntil:it("throws notAuthenticated when user is not logged in") { waitUntil { done in Task { do { _ = try await entitlementsService.fetchEntitlements() fail("Expected error to be thrown") } catch { expect(error).to(matchError(AuthError.notAuthenticated)) } done() } } }
🧹 Nitpick comments (12)
Sources/KindeSDK/Models/RolesResponse.swift (1)
3-28: LGTM - Well-structured response model.The response model properly handles optional data and provides helpful utility methods. The use of
compactMapcorrectly filters out roles with null keys.Note: The
isValid()implementation is duplicated across RolesResponse, PermissionsResponse, and FeatureFlagsResponse. Consider extracting this to a protocol for consistency and maintainability.Sources/KindeSDK/APIs/RolesAPI.swift (2)
10-24: Address SwiftLint warning: preferstaticmethods.The
final public classwith only class methods pattern triggers SwiftLint warnings. Since RolesAPI has no instance state, consider one of these approaches:
- Use a caseless enum (prevents instantiation):
-final public class RolesAPI { +public enum RolesAPI { + + @available(macOS 10.15, iOS 13.0, tvOS 13.0, watchOS 6.0, *) + public static func getRoles() async throws -> RolesResponse { - final public class func getRoles() async throws -> RolesResponse {
- Or use static methods directly on the class and add a private initializer to prevent instantiation.
Note: Apply the same fix to PermissionsAPI and FeatureFlagsAPI for consistency.
Based on static analysis hints from SwiftLint.
26-46: Implementation is correct but highly duplicated.The request builder pattern is properly implemented with authentication required. However, this code is nearly identical to PermissionsAPI and FeatureFlagsAPI, differing only in the endpoint path and return type. Consider extracting a generic request builder helper to reduce duplication.
Sources/KindeSDK/Models/PermissionsResponse.swift (1)
1-52: LGTM - Consistent with RolesResponse pattern.PermissionsResponse follows the same well-structured pattern as RolesResponse. As noted in the RolesResponse review, the
isValid()method is duplicated across all response models (Permissions, Roles, FeatureFlags). Consider extracting this to a protocol for better maintainability.Tests/EntitlementsSpec.swift (2)
349-355: Inconsistent indentation in nested describe blocks.The
getEntitlementsDictionaryandHard Check Methodsdescribe blocks have extra indentation compared to sibling blocks. This appears to be a formatting oversight.- describe("getEntitlementsDictionary") { - it("throws notAuthenticated when user is not logged in") { + describe("getEntitlementsDictionary") { + it("throws notAuthenticated when user is not logged in") {
41-42: TODO comments should be tracked for follow-up.Static analysis flagged unresolved TODO comments. Consider creating issues to track adding proper test fixtures with mocked tokens, or mark these tests as
pendingwith a clear reason.Would you like me to help create GitHub issues to track these test fixture improvements?
Also applies to: 263-264
Sources/KindeSDK/Models/FeatureFlagsResponse.swift (1)
37-47: Consider logging unknown flag types for debugging.Unknown flag types are silently skipped, which is safe but may make debugging difficult if the API introduces new types.
let flagType: Flag.ValueType? switch item.type { case "Boolean": flagType = .bool case "String": flagType = .string case "Integer": flagType = .int default: + // Silently skip unknown types - consider logging in debug builds continue }Sources/KindeSDK/Auth/Auth.swift (5)
1046-1073: DuplicateValueTypeenum definitions inFlagandFeatureFlag.Both
FlagandFeatureFlagstructs define identicalValueTypeenums with the same raw values and behavior. This duplication increases maintenance burden and risks divergence.Consider extracting a shared type:
/// Shared enum for feature flag value types public enum FlagValueType: String, Codable { case string = "s" case int = "i" case bool = "b" public var typeDescription: String { switch self { case .string: return "string" case .bool: return "boolean" case .int: return "integer" } } }Then update both structs to use
FlagValueTypeinstead of their nested enums.Also applies to: 1089-1124
1407-1415: Redundant nil initialization.SwiftLint correctly flags that
var startingAfter: String? = nilis redundant since optionals default tonil.public func getAllEntitlements() async throws -> [Entitlement] { var allEntitlements: [Entitlement] = [] - var startingAfter: String? = nil + var startingAfter: String? repeat { let response = try await fetchEntitlements(startingAfter: startingAfter)
1337-1343: HTTP requests lack timeout configuration.The
URLRequestobjects don't settimeoutInterval, relying on system defaults (60 seconds). For production SDK usage, explicit timeouts provide better control over failure modes.var request = URLRequest(url: url) request.httpMethod = "GET" + request.timeoutInterval = 30 // Explicit timeout request.setValue("Bearer \(token)", forHTTPHeaderField: "Authorization") request.setValue("application/json", forHTTPHeaderField: "Accept")Also applies to: 1378-1384
1219-1224:ClaimsService.getPermissionreturn type inconsistent withAuth.getPermission.
ClaimsService.getPermission(name:)returnsBool, whileAuth.getPermission(name:)returnsPermission?. This inconsistency may confuse SDK users.Consider aligning the return types:
/// Check if a specific permission is granted /// - Parameter name: The permission name to check - /// - Returns: True if permission is granted, false otherwise - public func getPermission(name: String) -> Bool { - return auth.getPermission(name: name) != nil + /// - Returns: Permission if found, nil otherwise + public func getPermission(name: String) -> Permission? { + return auth.getPermission(name: name) }Or rename to
hasPermissionif boolean return is intentional.
174-196: Async overloads throw generic NSError instead of typed AuthError.The async methods with
ApiOptionsthrowNSErrorwith domain "KindeSDK" for API failures, while other parts of the codebase use typedAuthErrorcases. This inconsistency makes error handling less ergonomic.Consider adding new
AuthErrorcases and using them consistently:public enum AuthError: Error { // existing cases... case apiError(String) case permissionsNotFound case rolesNotFound }Then:
- throw NSError(domain: "KindeSDK", code: -1, userInfo: [NSLocalizedDescriptionKey: "Failed to fetch permissions from API - check network and authentication"]) + throw AuthError.apiError("Failed to fetch permissions from API")Also applies to: 214-231, 250-272, 290-307
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
kinde-auth.jsonis excluded by!**/*.json
📒 Files selected for processing (13)
.swiftpm/xcode/package.xcworkspace/contents.xcworkspacedata(1 hunks)KindeSDK.xcodeproj/project.pbxproj(8 hunks)Sources/KindeSDK/APIs.swift(1 hunks)Sources/KindeSDK/APIs/FeatureFlagsAPI.swift(1 hunks)Sources/KindeSDK/APIs/PermissionsAPI.swift(1 hunks)Sources/KindeSDK/APIs/RolesAPI.swift(1 hunks)Sources/KindeSDK/Auth/Auth.swift(12 hunks)Sources/KindeSDK/Auth/Config.swift(1 hunks)Sources/KindeSDK/Models/FeatureFlagsResponse.swift(1 hunks)Sources/KindeSDK/Models/PermissionsResponse.swift(1 hunks)Sources/KindeSDK/Models/RolesResponse.swift(1 hunks)Tests/AuthSpec.swift(4 hunks)Tests/EntitlementsSpec.swift(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- .swiftpm/xcode/package.xcworkspace/contents.xcworkspacedata
- Sources/KindeSDK/Auth/Config.swift
🧰 Additional context used
🧬 Code graph analysis (6)
Sources/KindeSDK/APIs/RolesAPI.swift (5)
Sources/KindeSDK/APIs.swift (2)
macOS(60-83)execute(55-58)Sources/KindeSDK/APIs/FeatureFlagsAPI.swift (1)
macOS(21-24)Sources/KindeSDK/APIs/PermissionsAPI.swift (1)
macOS(21-24)Sources/KindeSDK/Auth/Auth.swift (3)
getRoles(250-272)getRoles(276-288)getRoles(1228-1230)Sources/KindeSDK/Utils/APIHelper.swift (1)
rejectNilHeaders(23-33)
Tests/EntitlementsSpec.swift (3)
Tests/AuthSpec.swift (1)
spec(7-200)Sources/KindeSDK/APIs.swift (1)
configure(120-153)Sources/KindeSDK/Auth/Auth.swift (26)
getEntitlements(1262-1286)getEntitlement(1291-1294)getBooleanEntitlement(1441-1451)getStringEntitlement(1458-1468)getNumericEntitlement(1475-1485)performHardCheck(1493-1500)validatePermission(1507-1513)validateRole(1520-1528)validateFeatureFlag(1535-1543)validateEntitlement(1550-1561)isUserAuthenticated(1565-1567)getUserOrganization(1571-1576)getUserSubscriptionTier(1580-1582)getFeatureFlags(1597-1621)isFeatureEnabled(1636-1652)getFeatureFlag(1626-1629)getRoles(250-272)getRoles(276-288)getRoles(1228-1230)getRole(290-307)getRole(312-324)getRole(1235-1237)fetchEntitlements(1311-1361)fetchEntitlement(1366-1402)getAllEntitlements(1407-1418)getEntitlementsDictionary(1423-1432)
Sources/KindeSDK/Models/FeatureFlagsResponse.swift (2)
Sources/KindeSDK/Models/PermissionsResponse.swift (1)
isValid(15-17)Sources/KindeSDK/Models/RolesResponse.swift (1)
isValid(15-17)
Sources/KindeSDK/APIs/FeatureFlagsAPI.swift (3)
Sources/KindeSDK/APIs.swift (2)
macOS(60-83)execute(55-58)Sources/KindeSDK/Auth/Auth.swift (1)
getFeatureFlags(1597-1621)Sources/KindeSDK/Utils/APIHelper.swift (1)
rejectNilHeaders(23-33)
Sources/KindeSDK/Models/PermissionsResponse.swift (2)
Sources/KindeSDK/Models/FeatureFlagsResponse.swift (1)
isValid(15-17)Sources/KindeSDK/Models/RolesResponse.swift (1)
isValid(15-17)
Sources/KindeSDK/Auth/Auth.swift (3)
Sources/KindeSDK/Models/PermissionsResponse.swift (1)
getPermissionKeys(21-27)Sources/KindeSDK/Models/RolesResponse.swift (1)
getRoleKeys(21-27)Sources/KindeSDK/Models/FeatureFlagsResponse.swift (1)
toFlagMap(22-53)
🪛 SwiftLint (0.57.0)
Sources/KindeSDK/APIs/RolesAPI.swift
[Warning] 21-21: Prefer static over final class
(static_over_final_class)
[Warning] 31-31: Prefer static over final class
(static_over_final_class)
Sources/KindeSDK/APIs/PermissionsAPI.swift
[Warning] 21-21: Prefer static over final class
(static_over_final_class)
[Warning] 31-31: Prefer static over final class
(static_over_final_class)
Tests/EntitlementsSpec.swift
[Warning] 41-41: TODOs should be resolved (Add proper test fixtures with ...)
(todo)
[Warning] 263-263: TODOs should be resolved (Add proper test fixtures with ...)
(todo)
Sources/KindeSDK/APIs/FeatureFlagsAPI.swift
[Warning] 21-21: Prefer static over final class
(static_over_final_class)
[Warning] 31-31: Prefer static over final class
(static_over_final_class)
Sources/KindeSDK/Auth/Auth.swift
[Warning] 1409-1409: Initializing an optional variable with nil is redundant
(redundant_optional_initialization)
🔇 Additional comments (14)
Sources/KindeSDK/APIs.swift (1)
115-115: LGTM - Documentation improvement.The updated terminology "Kinde SDK APIs" more accurately describes the SDK's scope.
Sources/KindeSDK/Models/RolesResponse.swift (2)
30-41: LGTM - Correct JSON mapping.The CodingKeys properly map
org_codefrom the API response toorgCodein Swift.
43-51: LGTM - Appropriate model structure.Optional properties correctly handle potentially missing fields from the API response.
KindeSDK.xcodeproj/project.pbxproj (2)
23-28: LGTM - New API and model files properly integrated.The new API clients (PermissionsAPI, RolesAPI, FeatureFlagsAPI) and their corresponding response models are correctly added to the project structure, groups, and build phases.
Also applies to: 93-98, 234-236, 284-286, 432-437
64-65: These files do not exist in the repository.Claims.swift and MobileEntitlements.swift are not present in the project and have no references in the project.pbxproj file. The review comment references code that does not exist in the current state of the codebase.
Likely an incorrect or invalid review comment.
Tests/AuthSpec.swift (4)
9-15: LGTM - Proper test setup.Adding
beforeEachto configure KindeSDKAPI ensures consistent test state. The API method name change fromisAuthorized()toisAuthenticated()appears intentional.
50-51: Verify error type change fromnotFoundtounknownError.The test expectations changed from
FlagError.notFoundtoFlagError.unknownErroracross multiple flag retrieval scenarios. This is a significant behavioral change that could affect existing code.Please confirm:
- Is this change intentional?
- Does this align with new error handling semantics?
- Are there any breaking changes for existing SDK users?
Also applies to: 55-56, 65-66, 75-76
99-177: LGTM - Good test coverage for new async APIs.The new test suites properly cover:
- ApiOptions initialization and default values
- getAllFlags behavior when not authenticated
- async API methods with forceApi option
The use of
waitUntilwith timeout andpendingplaceholders for authenticated scenarios is appropriate.
179-198: LGTM - Consistent async test patterns.The async flag tests follow the established testing patterns and properly handle default values and error cases.
Tests/EntitlementsSpec.swift (1)
6-16: Setup looks reasonable for integration-style tests.The test setup initializes the SDK and obtains service references from the shared auth instance. This approach works for basic smoke tests but limits testability since there's no mock injection.
Sources/KindeSDK/Models/FeatureFlagsResponse.swift (1)
1-54: Well-structured response model following established patterns.The
FeatureFlagsResponseimplementation correctly mirrors the patterns used inPermissionsResponseandRolesResponse. ThetoFlagMap()method properly validates keys and values before constructingFlagobjects.Sources/KindeSDK/Auth/Auth.swift (3)
94-104: Lazy service properties provide convenient access.The lazy initialization pattern ensures services are only created when accessed and maintains a single instance per Auth object.
991-1044: AnyCodable now properly handles arrays, dictionaries, and null.The implementation correctly decodes nested structures and handles
NSNullfor nil values, addressing the previous review concern about scalar-only support.
1631-1652: This review comment is incorrect and should be dismissed.The methods
isFeatureEnabled()andgetFeatureFlag()do not exist in the codebase. The code snippet shown (lines 1631-1652) also does not exist in Auth.swift, which contains only 704 lines total. Additionally,EntitlementsSpec.swiftdoes not exist in the repository.The actual feature flag methods in Auth.swift are
getFlag(),getBooleanFlag(),getStringFlag(), andgetIntegerFlag(), all of which consistently use thecode:parameter. Parameter naming is already consistent and there is no issue to address.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
Tests/EntitlementsSpec.swift (2)
269-277:getClaimValue(forKey:)tests may not compile or will always fail without fixtures.Two separate issues to double‑check here:
API existence: From the surrounding context,
ClaimsServiceclearly exposesgetClaim(forKey:), but it’s not obvious that agetClaimValue(forKey:)helper actually exists. If it doesn’t, these tests won’t compile. Either:
- add
getClaimValue(forKey:)toClaimsService(likely returningAny?from the underlyingClaim), or- rewrite the tests to use
getClaim(forKey:)and assert on theClaim’s value.Fixture setup: The
"returns claim value when claim exists"example uses"test_claim"without seeding any token/claim, so the result will benilin the current setup andexpect(result).to(beAKindOf(Any.self))will fail. As with the entitlements tests you already markedpending, you’ll need either a mocked token with that claim or to mark this examplependinguntil fixtures are added.#!/bin/bash # Verify whether getClaimValue is actually defined anywhere. rg -n "getClaimValue" Sources Tests -C2
321-355: Asyncawait expect { try await … }blocks inQuickSpecare likely to fail to compile.These
itblocks callawait expect { try await … }butEntitlementsSpeccurrently subclassesQuickSpecandspec()itself is notasync. Unless you’re on a Quick version whereQuickSpecsupports asyncitclosures, this will trigger the usual'async' call in a function that does not support concurrencybuild error.Options:
- Switch this spec to an async variant (e.g.,
AsyncSpec) and makespec()async so you can safelyawaitinsideitclosures, or- Keep
QuickSpecbut wrap the async calls using a pattern likewaitUntil/toEventually, or test yourAuth/entitlements HTTP error cases via synchronous stubs instead of real async functions.Quick & Nimble async: For a Swift test using Quick, is `await expect { try await foo() }` supported inside a `QuickSpec` `it` closure, or must the spec subclass `AsyncSpec` / use an async `spec()` instead?
🧹 Nitpick comments (2)
Tests/EntitlementsSpec.swift (2)
41-42: Align TODO-style comments with SwiftLint and your pending tests.SwiftLint is warning on these
// TODO: Add proper test fixtures…comments. Since the corresponding examples are already markedpending, you could either:
- remove or rephrase the TODO (e.g., reference a ticket ID instead), or
- adjust your SwiftLint config if you intentionally track work via TODOs in tests.
This keeps the lint signal focused on actionable issues.
Also applies to: 264-265
57-61: Several tests assert only on static types/fallbacks without seeding entitlements/flags.In these examples (string-bool/string-numeric entitlements, malformed/null/type-conversion edge cases, hard-check permission/role/flag/entitlement, and “feature flags dictionary when claim exists”), no token/claims/entitlements are set up. The assertions effectively only confirm:
- the functions return their declared types, or
- the documented default value paths work when nothing is present.
They do not currently exercise the “string value”, “malformed claim”, or “claim exists” branches suggested by the test names and comments.
Consider either:
- seeding a mock token / entitlements / feature flags for the specific keys under test, or
- marking these as
pending(or renaming them) until fixtures exist, to avoid a misleading sense of coverage.Also applies to: 76-80, 95-99, 121-124, 126-129, 131-138, 160-165, 172-180, 214-217
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Tests/EntitlementsSpec.swift(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
Tests/EntitlementsSpec.swift (1)
Sources/KindeSDK/Auth/Auth.swift (28)
getEntitlements(1262-1286)getEntitlement(1291-1294)getBooleanEntitlement(1441-1451)getStringEntitlement(1458-1468)getNumericEntitlement(1475-1485)performHardCheck(1493-1500)validatePermission(1507-1513)validateRole(1520-1528)validateFeatureFlag(1535-1543)validateEntitlement(1550-1561)isUserAuthenticated(1565-1567)getUserOrganization(1571-1576)getUserSubscriptionTier(1580-1582)getFeatureFlags(1597-1621)isFeatureEnabled(1636-1652)getFeatureFlag(1626-1629)getClaim(148-159)getClaim(1215-1217)getRoles(250-272)getRoles(276-288)getRoles(1228-1230)getRole(290-307)getRole(312-324)getRole(1235-1237)fetchEntitlements(1311-1361)fetchEntitlement(1366-1402)getAllEntitlements(1407-1418)getEntitlementsDictionary(1423-1432)
🪛 SwiftLint (0.57.0)
Tests/EntitlementsSpec.swift
[Warning] 41-41: TODOs should be resolved (Add proper test fixtures with ...)
(todo)
[Warning] 264-264: TODOs should be resolved (Add proper test fixtures with ...)
(todo)
🔇 Additional comments (1)
Tests/EntitlementsSpec.swift (1)
18-120: Nice structural coverage of EntitlementsService behaviors.The spec does a good job covering empty vs non-existent entitlements, default values, and the basic
performHardCheckbehavior. This gives you a solid baseline around the public surface, with more detailed happy-path checks to be filled in once fixtures/tokens are available.
|
Code updates look good. Testing next. |
victoreronmosele
left a comment
victoreronmosele
left a comment
There was a problem hiding this comment.
Let's remove references to Package.resolved since we're ignoring it in dff54f1 and it's prevent successful builds.
I highlighted the lines below.
Addresses review comments from victoreronmosele: - Remove PBXBuildFile entry for Package.resolved - Remove PBXFileReference entry for Package.resolved - Remove from main group - Remove from Resources build phase Since Package.resolved is now in .gitignore, it should not be referenced in the Xcode project.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
KindeSDK.xcodeproj/project.pbxproj (1)
126-127: Consider adding Claims and MobileEntitlements to a visible group
Claims.swiftandMobileEntitlements.swifthavePBXFileReferenceentries and are compiled, but they don’t appear under any PBXGroup’schildren. Adding them (likely to theAuthgroup) would make them visible in the Xcode navigator and easier to maintain.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
KindeSDK.xcodeproj/project.pbxproj(8 hunks)
🔇 Additional comments (7)
KindeSDK.xcodeproj/project.pbxproj (7)
22-27: New API and response PBXBuildFile entries are correctly wiredEach of the new
Permissions*,Roles*, andFeatureFlags*entries has a validfileRefand is later included in the Sources build phase, so the framework will compile these new files as expected.
63-64: Claims and MobileEntitlements build file entries are now correct
Claims.swiftandMobileEntitlements.swiftare defined asPBXBuildFileentries targeting Sources (not Frameworks), matching their use later in the main target’sPBXSourcesBuildPhase. This fixes the previous wiring issue.
91-96: File references for new APIs and responses look consistentThe
PBXFileReferencedefinitions forPermissionsAPI.swift,RolesAPI.swift,FeatureFlagsAPI.swiftand the three*Response.swiftmodels are well‑formed and align with their corresponding build files and group placements.
231-233: APIs group updated appropriately for new endpointsThe
APIsgroup now includesPermissionsAPI.swift,RolesAPI.swift, andFeatureFlagsAPI.swiftalongsideOAuthAPI.swift, matching the new file references and build entries.
281-283: Models group correctly includes new response types
PermissionsResponse.swift,RolesResponse.swift, andFeatureFlagsResponse.swiftare added under theModelsgroup, consistent with their file references and inclusion in the Sources build phase.
425-433: New API and response sources are properly added to the main targetAll newly introduced API and response files are listed in the
KindeSDKtarget’sPBXSourcesBuildPhase, so they will be compiled into the framework. No duplicate entries or obvious ordering/syntax issues here.
455-456: Claims and MobileEntitlements are now part of the KindeSDK Sources build phase
Claims.swiftandMobileEntitlements.swiftare correctly added to the main target’sSourcesbuild phase, with a single entry each and no lingering framework-phase wiring. This should resolve prior build issues around these files.
These files don't exist in the project and were causing build failures. Removed all references from: - PBXBuildFile section - PBXFileReference section - Sources build phase Addresses review comment from victoreronmosele.
victoreronmosele
left a comment
victoreronmosele
left a comment
There was a problem hiding this comment.
We need to add the EntitlementSpec file to the XCode project to run and verify the tests.
Here are some suggested changes.
| 3B41B1FB2A0E5F9100EFD9E4 /* FeatureFlagsAPI.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3B41B1FA2A0E5F9100EFD9E4 /* FeatureFlagsAPI.swift */; }; | ||
| 3B41B1FD2A0E5F9100EFD9E4 /* PermissionsResponse.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3B41B1FC2A0E5F9100EFD9E4 /* PermissionsResponse.swift */; }; | ||
| 3B41B1FF2A0E5F9100EFD9E4 /* RolesResponse.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3B41B1FE2A0E5F9100EFD9E4 /* RolesResponse.swift */; }; | ||
| 3B41B2012A0E5F9100EFD9E4 /* FeatureFlagsResponse.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3B41B2002A0E5F9100EFD9E4 /* FeatureFlagsResponse.swift */; }; |
There was a problem hiding this comment.
| 3B41B2012A0E5F9100EFD9E4 /* FeatureFlagsResponse.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3B41B2002A0E5F9100EFD9E4 /* FeatureFlagsResponse.swift */; }; | |
| 3B41B2012A0E5F9100EFD9E4 /* FeatureFlagsResponse.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3B41B2002A0E5F9100EFD9E4 /* FeatureFlagsResponse.swift */; }; | |
| 8D6DE2E32EE0F34000476C3F /* EntitlementsSpec.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8D6DE2E22EE0F34000476C3F /* EntitlementsSpec.swift */; }; |
| 3B41B1FA2A0E5F9100EFD9E4 /* FeatureFlagsAPI.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = FeatureFlagsAPI.swift; sourceTree = "<group>"; }; | ||
| 3B41B1FC2A0E5F9100EFD9E4 /* PermissionsResponse.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = PermissionsResponse.swift; sourceTree = "<group>"; }; | ||
| 3B41B1FE2A0E5F9100EFD9E4 /* RolesResponse.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = RolesResponse.swift; sourceTree = "<group>"; }; | ||
| 3B41B2002A0E5F9100EFD9E4 /* FeatureFlagsResponse.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = FeatureFlagsResponse.swift; sourceTree = "<group>"; }; |
There was a problem hiding this comment.
| 3B41B2002A0E5F9100EFD9E4 /* FeatureFlagsResponse.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = FeatureFlagsResponse.swift; sourceTree = "<group>"; }; | |
| 3B41B2002A0E5F9100EFD9E4 /* FeatureFlagsResponse.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = FeatureFlagsResponse.swift; sourceTree = "<group>"; }; | |
| 8D6DE2E22EE0F34000476C3F /* EntitlementsSpec.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = EntitlementsSpec.swift; sourceTree = "<group>"; }; |
| children = ( | ||
| 3B41B1AD2A0E5F8500EFD9E4 /* AuthSpec.swift */, | ||
| 3B41B1AE2A0E5F8500EFD9E4 /* StringJWTTokenDecodeSpec.swift */, | ||
| 3B41B1AF2A0E5F8500EFD9E4 /* AuthErrorSpec.swift */, | ||
| 3B41B1B02A0E5F8500EFD9E4 /* DefaultLoggerSpec.swift */, | ||
| 3B41B1B12A0E5F8500EFD9E4 /* ConfigSpec.swift */, | ||
| ); |
There was a problem hiding this comment.
These should include the EntitlementSpec file.
Please update to:
children = (
+ 8D6DE2E22EE0F34000476C3F /* EntitlementsSpec.swift */,
3B41B1AD2A0E5F8500EFD9E4 /* AuthSpec.swift */,
3B41B1AE2A0E5F8500EFD9E4 /* StringJWTTokenDecodeSpec.swift */,
3B41B1AF2A0E5F8500EFD9E4 /* AuthErrorSpec.swift */,
3B41B1B02A0E5F8500EFD9E4 /* DefaultLoggerSpec.swift */,
3B41B1B12A0E5F8500EFD9E4 /* ConfigSpec.swift */,
);| files = ( | ||
| 3B41B1B32A0E5F8500EFD9E4 /* StringJWTTokenDecodeSpec.swift in Sources */, | ||
| 3B41B1B52A0E5F8500EFD9E4 /* DefaultLoggerSpec.swift in Sources */, | ||
| 3B41B1B62A0E5F8500EFD9E4 /* ConfigSpec.swift in Sources */, | ||
| 3B41B1B42A0E5F8500EFD9E4 /* AuthErrorSpec.swift in Sources */, | ||
| 3B41B1B22A0E5F8500EFD9E4 /* AuthSpec.swift in Sources */, | ||
| 3BA67F872A0A5FF600399C6A /* kinde-auth.json in Sources */, | ||
| ); |
There was a problem hiding this comment.
These should include the EntitlementSpec file.
Please update to:
files = (
+ 8D6DE2E32EE0F34000476C3F /* EntitlementsSpec.swift in Sources */,
3B41B1B32A0E5F8500EFD9E4 /* StringJWTTokenDecodeSpec.swift in Sources */,
3B41B1B52A0E5F8500EFD9E4 /* DefaultLoggerSpec.swift in Sources */,
3B41B1B62A0E5F8500EFD9E4 /* ConfigSpec.swift in Sources */,
3B41B1B42A0E5F8500EFD9E4 /* AuthErrorSpec.swift in Sources */,
3B41B1B22A0E5F8500EFD9E4 /* AuthSpec.swift in Sources */,
3BA67F872A0A5FF600399C6A /* kinde-auth.json in Sources */,
);
victoreronmosele
left a comment
victoreronmosele
left a comment
There was a problem hiding this comment.
These changes are needed to get the EntitlementsSpec tests to compile and pass.
| it("maintains consistent state with auth service") { | ||
| let entitlements1 = auth.entitlements.getEntitlements() | ||
| let entitlements2 = entitlementsService.getEntitlements() | ||
| expect(entitlements1).to(equal(entitlements2)) |
There was a problem hiding this comment.
The current check has a compile error: Type 'Any' cannot conform to 'Equatable' since the entitlements are of type [String : Any].
We can cast the entitlements to NSDictionary to remove the error.
| expect(entitlements1).to(equal(entitlements2)) | |
| expect(entitlements1 as NSDictionary).to(equal(entitlements2 as NSDictionary)) |
| let result = claimsService.getClaimValue(forKey: "test_claim") | ||
| expect(result).to(beAKindOf(Any.self)) |
There was a problem hiding this comment.
This tests doesn't pass as it requires the mock token just like its similar test for the getClaim method.
We can add this pending note here.
| let result = claimsService.getClaimValue(forKey: "test_claim") | |
| expect(result).to(beAKindOf(Any.self)) | |
| // This would require a mock token with specific claims | |
| // TODO: Add proper test fixtures with mocked tokens | |
| pending("Requires mock token with claims") {} |
| import KindeSDK | ||
| import Foundation | ||
|
|
||
| class EntitlementsSpec: QuickSpec { |
There was a problem hiding this comment.
The tests for fetchEntitlements and getAllEntitlements have async functions and require using the AsyncSpec to be able to run the tests without a compilation error.
| class EntitlementsSpec: QuickSpec { | |
| class EntitlementsSpec: AsyncSpec { |
2 participants
Explain your changes
Entitlements and Hardcheck
Checklist
🛟 If you need help, consider asking for advice over in the Kinde community.