-
Notifications
You must be signed in to change notification settings - Fork 0
Generate proto files to support given_name and family_name in user object #29
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughSDK version bumped from 2.0.5 to 2.0.6. Protobuf audit log messages extended with resource filtering fields. Session management service removed. Auth service updated with new GetAuthError RPC. WebAuthn-related fields and messages added to authentication API. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant AuthService as Auth Service
participant WebAuthn as WebAuthn Protocol
rect rgb(200, 220, 255)
Note over Client,WebAuthn: WebAuthn Authentication Flow (New)
Client->>AuthService: BeginAuthenticationRequest(email)
AuthService->>AuthService: Generate Challenge & Session
AuthService-->>Client: BeginAuthenticationResponse(sessionId, options)
Client->>WebAuthn: Authenticate with Challenge
WebAuthn-->>Client: AuthenticatorAssertionResponse
Client->>AuthService: FinishAuthenticationRequest(sessionId, assertion)
AuthService->>AuthService: Verify Assertion & Signature
alt Success
AuthService-->>Client: FinishAuthenticationResponse(success, userId, sessionToken)
else Error
AuthService-->>Client: GetAuthErrorResponse(error, description)
end
end
rect rgb(220, 200, 255)
Note over Client,WebAuthn: WebAuthn Registration Flow (New)
Client->>AuthService: BeginRegistrationRequest()
AuthService->>AuthService: Generate Challenge & Session
AuthService-->>Client: BeginRegistrationResponse(sessionId, options)
Client->>WebAuthn: Register Credential
WebAuthn-->>Client: AttestationObject & ClientDataJson
Client->>AuthService: FinishRegistrationRequest(sessionId, attestation, clientData)
AuthService->>AuthService: Verify Attestation
alt Success
AuthService-->>Client: FinishRegistrationResponse(success)
else Error
AuthService-->>Client: DeleteCredentialResponse with UnknownCredentialOptions
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Areas requiring extra attention:
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
README.md (1)
56-69: Fix Maven version inconsistency in README.The Gradle dependency (line 56) is updated to 2.0.6, but the Maven snippet (line 67) still references 2.0.5. This inconsistency will cause Maven users to install an outdated version. Both examples must match the current release version.
Apply this diff to update the Maven version:
<dependency> <groupId>com.scalekit</groupId> <artifactId>scalekit-sdk-java</artifactId> - <version>2.0.5</version> + <version>2.0.6</version> </dependency>
🧹 Nitpick comments (1)
src/main/java/com/scalekit/grpc/scalekit/v1/auth/webauthn/DeleteCredentialRequest.java (1)
1-10: PR title mentions "given_name and family_name in user object" but this file adds WebAuthn credential deletion.This file appears unrelated to the user name fields mentioned in the PR title. If this is part of a broader protobuf regeneration that includes multiple unrelated changes, consider updating the PR title to reflect the full scope (e.g., "Update protobuf definitions for user fields and WebAuthn support").
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (63)
README.md(1 hunks)pom.xml(1 hunks)src/main/java/com/scalekit/grpc/scalekit/v1/auditlogs/AuditlogsProto.java(3 hunks)src/main/java/com/scalekit/grpc/scalekit/v1/auditlogs/AuthLogRequest.java(11 hunks)src/main/java/com/scalekit/grpc/scalekit/v1/auditlogs/AuthLogRequestOrBuilder.java(1 hunks)src/main/java/com/scalekit/grpc/scalekit/v1/auditlogs/ListAuthLogRequest.java(11 hunks)src/main/java/com/scalekit/grpc/scalekit/v1/auditlogs/ListAuthLogRequestOrBuilder.java(1 hunks)src/main/java/com/scalekit/grpc/scalekit/v1/auditlogs/LocationOrBuilder.java(0 hunks)src/main/java/com/scalekit/grpc/scalekit/v1/auditlogs/RevokeSessionRequestOrBuilder.java(0 hunks)src/main/java/com/scalekit/grpc/scalekit/v1/auditlogs/RevokeSessionResponseOrBuilder.java(0 hunks)src/main/java/com/scalekit/grpc/scalekit/v1/auditlogs/SessionDetailsOrBuilder.java(0 hunks)src/main/java/com/scalekit/grpc/scalekit/v1/auditlogs/SessionDetailsRequestOrBuilder.java(0 hunks)src/main/java/com/scalekit/grpc/scalekit/v1/auditlogs/SessionServiceGrpc.java(0 hunks)src/main/java/com/scalekit/grpc/scalekit/v1/auditlogs/SessionsProto.java(0 hunks)src/main/java/com/scalekit/grpc/scalekit/v1/auditlogs/UserSessionDetailsOrBuilder.java(0 hunks)src/main/java/com/scalekit/grpc/scalekit/v1/auditlogs/UserSessionDetailsRequest.java(0 hunks)src/main/java/com/scalekit/grpc/scalekit/v1/auditlogs/UserSessionDetailsRequestOrBuilder.java(0 hunks)src/main/java/com/scalekit/grpc/scalekit/v1/auditlogs/UserSessionFilter.java(0 hunks)src/main/java/com/scalekit/grpc/scalekit/v1/auditlogs/UserSessionFilterOrBuilder.java(0 hunks)src/main/java/com/scalekit/grpc/scalekit/v1/auth/AuthMethod.java(10 hunks)src/main/java/com/scalekit/grpc/scalekit/v1/auth/AuthMethodOrBuilder.java(1 hunks)src/main/java/com/scalekit/grpc/scalekit/v1/auth/AuthProto.java(5 hunks)src/main/java/com/scalekit/grpc/scalekit/v1/auth/AuthServiceGrpc.java(9 hunks)src/main/java/com/scalekit/grpc/scalekit/v1/auth/AuthState.java(3 hunks)src/main/java/com/scalekit/grpc/scalekit/v1/auth/GetAuthErrorRequest.java(13 hunks)src/main/java/com/scalekit/grpc/scalekit/v1/auth/GetAuthErrorRequestOrBuilder.java(1 hunks)src/main/java/com/scalekit/grpc/scalekit/v1/auth/GetAuthErrorResponse.java(1 hunks)src/main/java/com/scalekit/grpc/scalekit/v1/auth/GetAuthErrorResponseOrBuilder.java(1 hunks)src/main/java/com/scalekit/grpc/scalekit/v1/auth/passwordless/PasswordlessProto.java(3 hunks)src/main/java/com/scalekit/grpc/scalekit/v1/auth/webauthn/AllAcceptedCredentialsOptions.java(1 hunks)src/main/java/com/scalekit/grpc/scalekit/v1/auth/webauthn/AllAcceptedCredentialsOptionsOrBuilder.java(1 hunks)src/main/java/com/scalekit/grpc/scalekit/v1/auth/webauthn/AuthenticatorAssertionResponse.java(1 hunks)src/main/java/com/scalekit/grpc/scalekit/v1/auth/webauthn/AuthenticatorAssertionResponseOrBuilder.java(1 hunks)src/main/java/com/scalekit/grpc/scalekit/v1/auth/webauthn/BeginAuthenticationRequest.java(1 hunks)src/main/java/com/scalekit/grpc/scalekit/v1/auth/webauthn/BeginAuthenticationRequestOrBuilder.java(1 hunks)src/main/java/com/scalekit/grpc/scalekit/v1/auth/webauthn/BeginAuthenticationResponse.java(1 hunks)src/main/java/com/scalekit/grpc/scalekit/v1/auth/webauthn/BeginAuthenticationResponseOrBuilder.java(1 hunks)src/main/java/com/scalekit/grpc/scalekit/v1/auth/webauthn/BeginRegistrationRequest.java(1 hunks)src/main/java/com/scalekit/grpc/scalekit/v1/auth/webauthn/BeginRegistrationRequestOrBuilder.java(1 hunks)src/main/java/com/scalekit/grpc/scalekit/v1/auth/webauthn/BeginRegistrationResponse.java(13 hunks)src/main/java/com/scalekit/grpc/scalekit/v1/auth/webauthn/BeginRegistrationResponseOrBuilder.java(1 hunks)src/main/java/com/scalekit/grpc/scalekit/v1/auth/webauthn/DeleteCredentialRequest.java(1 hunks)src/main/java/com/scalekit/grpc/scalekit/v1/auth/webauthn/DeleteCredentialRequestOrBuilder.java(1 hunks)src/main/java/com/scalekit/grpc/scalekit/v1/auth/webauthn/DeleteCredentialResponse.java(1 hunks)src/main/java/com/scalekit/grpc/scalekit/v1/auth/webauthn/DeleteCredentialResponseOrBuilder.java(1 hunks)src/main/java/com/scalekit/grpc/scalekit/v1/auth/webauthn/FinishAuthenticationRequest.java(1 hunks)src/main/java/com/scalekit/grpc/scalekit/v1/auth/webauthn/FinishAuthenticationRequestOrBuilder.java(1 hunks)src/main/java/com/scalekit/grpc/scalekit/v1/auth/webauthn/FinishAuthenticationResponse.java(1 hunks)src/main/java/com/scalekit/grpc/scalekit/v1/auth/webauthn/FinishAuthenticationResponseOrBuilder.java(1 hunks)src/main/java/com/scalekit/grpc/scalekit/v1/auth/webauthn/FinishRegistrationRequest.java(1 hunks)src/main/java/com/scalekit/grpc/scalekit/v1/auth/webauthn/FinishRegistrationRequestOrBuilder.java(1 hunks)src/main/java/com/scalekit/grpc/scalekit/v1/auth/webauthn/FinishRegistrationResponse.java(1 hunks)src/main/java/com/scalekit/grpc/scalekit/v1/auth/webauthn/FinishRegistrationResponseOrBuilder.java(1 hunks)src/main/java/com/scalekit/grpc/scalekit/v1/auth/webauthn/GetRelatedOriginsRequest.java(1 hunks)src/main/java/com/scalekit/grpc/scalekit/v1/auth/webauthn/GetRelatedOriginsRequestOrBuilder.java(1 hunks)src/main/java/com/scalekit/grpc/scalekit/v1/auth/webauthn/GetRelatedOriginsResponse.java(1 hunks)src/main/java/com/scalekit/grpc/scalekit/v1/auth/webauthn/GetRelatedOriginsResponseOrBuilder.java(1 hunks)src/main/java/com/scalekit/grpc/scalekit/v1/auth/webauthn/ListCredentialsRequest.java(1 hunks)src/main/java/com/scalekit/grpc/scalekit/v1/auth/webauthn/ListCredentialsRequestOrBuilder.java(1 hunks)src/main/java/com/scalekit/grpc/scalekit/v1/auth/webauthn/ListCredentialsResponse.java(1 hunks)src/main/java/com/scalekit/grpc/scalekit/v1/auth/webauthn/ListCredentialsResponseOrBuilder.java(1 hunks)src/main/java/com/scalekit/grpc/scalekit/v1/auth/webauthn/UnknownCredentialOptions.java(1 hunks)src/main/java/com/scalekit/grpc/scalekit/v1/auth/webauthn/UnknownCredentialOptionsOrBuilder.java(1 hunks)
💤 Files with no reviewable changes (12)
- src/main/java/com/scalekit/grpc/scalekit/v1/auditlogs/UserSessionFilterOrBuilder.java
- src/main/java/com/scalekit/grpc/scalekit/v1/auditlogs/RevokeSessionRequestOrBuilder.java
- src/main/java/com/scalekit/grpc/scalekit/v1/auditlogs/SessionsProto.java
- src/main/java/com/scalekit/grpc/scalekit/v1/auditlogs/RevokeSessionResponseOrBuilder.java
- src/main/java/com/scalekit/grpc/scalekit/v1/auditlogs/UserSessionDetailsRequestOrBuilder.java
- src/main/java/com/scalekit/grpc/scalekit/v1/auditlogs/UserSessionDetailsRequest.java
- src/main/java/com/scalekit/grpc/scalekit/v1/auditlogs/SessionDetailsOrBuilder.java
- src/main/java/com/scalekit/grpc/scalekit/v1/auditlogs/UserSessionDetailsOrBuilder.java
- src/main/java/com/scalekit/grpc/scalekit/v1/auditlogs/UserSessionFilter.java
- src/main/java/com/scalekit/grpc/scalekit/v1/auditlogs/SessionServiceGrpc.java
- src/main/java/com/scalekit/grpc/scalekit/v1/auditlogs/SessionDetailsRequestOrBuilder.java
- src/main/java/com/scalekit/grpc/scalekit/v1/auditlogs/LocationOrBuilder.java
🔇 Additional comments (26)
pom.xml (1)
10-10: Version bump aligns with proto changes.The SDK version increment from 2.0.5 to 2.0.6 appropriately reflects the protobuf schema extensions and service updates introduced in this PR (given_name/family_name fields, WebAuthn additions, auth service updates).
src/main/java/com/scalekit/grpc/scalekit/v1/auth/webauthn/FinishAuthenticationResponseOrBuilder.java (1)
1-40: Generated protobuf interface – no manual edits required.This is a protobuf-generated interface file that provides the accessor contract for
FinishAuthenticationResponse. The generated code follows standard protobuf patterns and should not be manually modified.src/main/java/com/scalekit/grpc/scalekit/v1/auth/webauthn/FinishAuthenticationResponse.java (1)
1-745: Generated protobuf message implementation – no manual edits required.This is a protobuf-generated message class that implements
FinishAuthenticationResponsewith standard serialization, parsing, builder pattern, and field accessors. The generated code follows protobuf Java conventions and should not be manually modified. Any changes should be made to the source.protofile and regenerated.src/main/java/com/scalekit/grpc/scalekit/v1/auth/webauthn/FinishRegistrationResponseOrBuilder.java (1)
1-16: LGTM! Generated protobuf interface is correct.This is a standard protobuf-generated interface file with the expected structure for a message containing a single boolean field.
src/main/java/com/scalekit/grpc/scalekit/v1/auth/webauthn/FinishRegistrationResponse.java (1)
1-473: LGTM! Generated protobuf message class is correct.This is a standard protobuf-generated message class with proper implementation of:
- Serialization/deserialization (writeTo, getSerializedSize, parseFrom methods)
- Builder pattern with bitField tracking
- Descriptor linkage to WebauthnProto
- Standard equals/hashCode implementation
- Proper unknown fields handling
The generated code follows all expected protobuf conventions for a message with a single boolean field.
src/main/java/com/scalekit/grpc/scalekit/v1/auth/webauthn/BeginRegistrationRequestOrBuilder.java (1)
1-10: Auto-generated protobuf code looks correct.This is standard protobuf-generated code for an empty message. The empty interface is expected when the corresponding protobuf message definition contains no fields.
src/main/java/com/scalekit/grpc/scalekit/v1/auth/webauthn/BeginRegistrationRequest.java (1)
1-399: Auto-generated protobuf code is valid, but AI summary is inconsistent.This is standard protobuf-generated code for an empty message definition. The implementation correctly handles serialization, parsing, and the builder pattern according to protobuf conventions.
However, the AI summary claims this class includes an "email field" with methods like
hasEmail(),getEmail(), andgetEmailBytes(), but no such field or methods exist in the actual code. The message is currently empty with no fields defined.src/main/java/com/scalekit/grpc/scalekit/v1/auth/webauthn/DeleteCredentialRequestOrBuilder.java (1)
11-21: Accessors match proto definitionStandard string/bytes getters look correct for
credential_id; nothing to fix.src/main/java/com/scalekit/grpc/scalekit/v1/auth/webauthn/UnknownCredentialOptionsOrBuilder.java (1)
11-33: OrBuilder contract is consistent
rp_idandcredential_idaccessors are wired exactly as expected; no issues spotted.src/main/java/com/scalekit/grpc/scalekit/v1/auth/webauthn/DeleteCredentialResponseOrBuilder.java (1)
11-30: Response OrBuilder fields look goodBoolean and nested message accessors align with the proto; this generated interface is solid.
src/main/java/com/scalekit/grpc/scalekit/v1/auth/webauthn/UnknownCredentialOptions.java (1)
44-613: Generated message aligns with UnknownCredentialOptions protoInitialization, serialization, and builder plumbing follow the usual protoc patterns; no adjustments needed.
src/main/java/com/scalekit/grpc/scalekit/v1/auth/webauthn/DeleteCredentialResponse.java (1)
44-597: DeleteCredentialResponse message looks consistentBit-field tracking, nested builder handling, and parse/write methods all match the proto definition; nothing to change.
src/main/java/com/scalekit/grpc/scalekit/v1/auth/webauthn/DeleteCredentialRequest.java (1)
1-541: LGTM! Protobuf-generated code is complete and follows standard conventions.This is auto-generated protobuf code (version 3.25.3) and follows the expected patterns for Java message classes. The implementation includes proper null checks, serialization, and builder methods for the
credential_idfield.src/main/java/com/scalekit/grpc/scalekit/v1/auth/webauthn/FinishRegistrationRequestOrBuilder.java (1)
1-92: Generated interface looks consistent.Accessor coverage mirrors the FinishRegistrationRequest schema; no issues spotted.
src/main/java/com/scalekit/grpc/scalekit/v1/auth/webauthn/FinishRegistrationRequest.java (1)
1-1260: Message implementation aligns with proto output.Constructor defaults, field accessors, and builder APIs match the WebAuthn definition; everything checks out.
src/main/java/com/scalekit/grpc/scalekit/v1/auth/webauthn/GetRelatedOriginsRequestOrBuilder.java (1)
1-10: Generated request OrBuilder matches empty message.Interface correctly extends
MessageOrBuilderwithout extra accessors, consistent with the empty proto definition.src/main/java/com/scalekit/grpc/scalekit/v1/auth/webauthn/GetRelatedOriginsResponseOrBuilder.java (1)
11-34: Response OrBuilder accessors are wired correctly.Accessor set covers list, count, element, and bytes variants exactly as expected for a repeated string field.
src/main/java/com/scalekit/grpc/scalekit/v1/auth/webauthn/GetRelatedOriginsResponse.java (1)
44-118: Repeated field plumbing looks standard.Field storage, serialization, and accessor logic mirror protoc’s generated pattern for
repeated string, with immutability handled viaLazyStringArrayList.src/main/java/com/scalekit/grpc/scalekit/v1/auth/webauthn/GetRelatedOriginsRequest.java (1)
53-94: Empty request message is generated correctly.Serialization, equality, and parsing logic match the standard template for a zero-field protobuf message.
src/main/java/com/scalekit/grpc/scalekit/v1/auditlogs/ListAuthLogRequestOrBuilder.java (1)
96-106: Resource accessor addition looks consistent.The new
resource_idstring/bytes accessors mirror the existing field patterns, so the OrBuilder surface stays in sync with the proto change.src/main/java/com/scalekit/grpc/scalekit/v1/auditlogs/AuditlogsProto.java (1)
51-121: Descriptor wiring matches the proto updates.
ListAuthLogRequestandAuthLogRequestdescriptors now exposeresource_id(and name/type), and the accessor tables list them in the correct order, so reflection clients keep working.src/main/java/com/scalekit/grpc/scalekit/v1/auditlogs/AuthLogRequestOrBuilder.java (1)
158-192: New resource getters follow the established pattern.String and ByteString accessors for
resource_id,resource_name, andresource_typeline up with the concrete message, keeping the OrBuilder API complete.src/main/java/com/scalekit/grpc/scalekit/v1/auditlogs/ListAuthLogRequest.java (1)
226-263:resource_idintegration looks thorough.Backing field, getters, serialization, hash/equals, and builder bit handling all reflect the new field number 7, so the message behaves as expected end-to-end.
src/main/java/com/scalekit/grpc/scalekit/v1/auditlogs/AuthLogRequest.java (1)
474-590: Resource metadata fields are wired correctly.The new strings plug into constructors, serialization, equality, hash, and builder bit masks, matching the proto definition for fields 12–14.
src/main/java/com/scalekit/grpc/scalekit/v1/auth/passwordless/PasswordlessProto.java (2)
66-251: No issues found—this is a bulk proto file regeneration PR.This is a bulk regeneration of all proto files, not a targeted change. The PR title is incomplete; the actual scope includes support for
given_name/family_name, WebAuthn operations, audit logging enhancements, and authentication error retrieval. The passwordless proto regeneration is expected and appropriate as part of this bulk update.Likely an incorrect or invalid review comment.
1-4: This review comment is based on a misunderstanding of the project's build workflow and should be disregarded.The repository is a generated SDK where:
- Source
.protofiles are stored in the gitignoredproto/directory- The
makefilerunsbuf generateto produce Java files from those protos- Only the generated Java files (
*Proto.java) are committed to version control- The
.protosource files do not appear in pull requests by designThe changes to
PasswordlessProto.javaand other generated files in this PR are correct and expected. No action is needed.Likely an incorrect or invalid review comment.
Summary by CodeRabbit
New Features
Chores