Split LoginByPassword into explicit RegisterByPassword and LoginByPassword#21
Merged
Merged
Conversation
…sword Agent-Logs-Url: https://github.com/poly-workshop/identra/sessions/64e8656e-2b91-4cda-88bf-84437b094797 Co-authored-by: slhmy <31381093+slhmy@users.noreply.github.com>
Copilot created this pull request from a session on behalf of
slhmy
April 15, 2026 04:28
View session
Contributor
There was a problem hiding this comment.
Pull request overview
This PR separates implicit “upsert” password login into two explicit flows—RegisterByPassword for account creation and LoginByPassword for authentication—making the API behavior unambiguous across gRPC/HTTP/OpenAPI.
Changes:
- Added
RegisterByPasswordrequest/response messages and a newRegisterByPasswordRPC (POST /password/register) to the proto API. - Implemented
Service.RegisterByPasswordand updatedService.LoginByPasswordto authenticate-only with clearer status codes. - Regenerated Go gRPC/gateway and OpenAPI outputs to reflect the new RPC.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| proto/identra/v1/types.proto | Adds RegisterByPasswordRequest/Response message types. |
| proto/identra/v1/identra_service.proto | Adds RegisterByPassword RPC and HTTP mapping. |
| internal/application/identra/service.go | Implements registration and updates password-login semantics. |
| gen/openapi/identra.swagger.json | Exposes /password/register in generated OpenAPI spec. |
| gen/go/identra/v1/types.pb.go | Regenerated Go types for new proto messages. |
| gen/go/identra/v1/identra_service_grpc.pb.go | Regenerated gRPC client/server stubs including new RPC. |
| gen/go/identra/v1/identra_service.pb.gw.go | Regenerated grpc-gateway handlers/routes for /password/register. |
| gen/go/identra/v1/identra_service.pb.go | Regenerated service descriptors and type mappings. |
Comment on lines
+535
to
+542
| func (s *Service) RegisterByPassword( | ||
| ctx context.Context, | ||
| req *identra_v1_pb.RegisterByPasswordRequest, | ||
| ) (*identra_v1_pb.RegisterByPasswordResponse, error) { | ||
| email := strings.TrimSpace(req.GetEmail()) | ||
| password := req.GetPassword() | ||
| if email == "" || password == "" { | ||
| return nil, status.Error(codes.InvalidArgument, "email and password are required") |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Contributor
|
@copilot apply changes based on the comments in this thread |
…registration/login Agent-Logs-Url: https://github.com/poly-workshop/identra/sessions/96f57d20-bfe5-46ba-9991-86c33cd07e7a Co-authored-by: slhmy <31381093+slhmy@users.noreply.github.com>
Contributor
Author
Done in commit 2e9e749. Duplicate-key race condition (
Unit tests (
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
LoginByPasswordwas doing implicit upsert semantics: creating users on first call and verifying credentials on subsequent calls. This conflates registration and authentication into a single ambiguous endpoint.Changes
Proto (
proto/identra/v1/)RegisterByPasswordRequest/RegisterByPasswordResponsemessages totypes.protoRegisterByPasswordRPC atPOST /password/registerinidentra_service.protoService (
internal/application/identra/service.go)RegisterByPassword: creates a new user; returnsAlreadyExistsif email is taken (including concurrent-registration races — duplicate-key errors from the persistence layer are translated toAlreadyExists)LoginByPassword: authenticates only; returnsNotFoundif no account,FailedPreconditionif password login isn't configured,Unauthenticatedfor wrong credentialsPersistence (
internal/infrastructure/persistence/)gorm_user_store.go:Createnow mapsgorm.ErrDuplicatedKey→domain.ErrAlreadyExistsmongo_user_store.go:Createnow mapsmongo.IsDuplicateKeyError()→domain.ErrAlreadyExistsDomain (
internal/domain/user.go)domain.ErrAlreadyExistssentinel errorGenerated (
gen/)Tests (
internal/application/identra/service_password_test.go)RegisterByPasswordandLoginByPassword, including the concurrent-registration race scenarioBehaviour comparison
RegisterByPassword: creates user;LoginByPassword:NOT_FOUNDRegisterByPassword:ALREADY_EXISTS;LoginByPassword:FAILED_PRECONDITIONLoginByPassword: issues tokenUNAUTHENTICATEDLoginByPassword:UNAUTHENTICATEDINTERNALRegisterByPassword:ALREADY_EXISTS