[BB-1459] Upgrade okta v5#122
Conversation
MarcusGoldschmidt
changed the title
MarcusGoldschmidt
changed the title
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
MarcusGoldschmidt
changed the title
MarcusGoldschmidt
force-pushed
the
goldschmidt/upgrade-role-okta-v5-groups
branch
from
bec281f to
acd32f8
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| if group.Profile == nil && group.Profile.Name == nil { | ||
| l.Warn("okta-aws-connector: group had nil profile or profile name, skipping", zap.Any("group", group)) | ||
| continue | ||
| } |
There was a problem hiding this comment.
Bug: Null Safety Logic Error Causes Panic
The null safety checks for group.Profile and group.Profile.Name incorrectly use && instead of ||. If group.Profile is nil, this leads to a nil pointer dereference panic when attempting to access group.Profile.Name.
Additional Locations (3)
|
|
||
| type RoleAssignments struct { | ||
| RoleAssignments []*RoleAssignment `json:"value,omitempty"` | ||
| RoleAssignments []*RoleAssignment `json:"Value,omitempty"` |
There was a problem hiding this comment.
Bug: API Breakage: JSON Field Case Change
The Value JSON tag in JSONPatchOperation and RoleAssignments structs changed from lowercase to uppercase. This breaks API compatibility, as JSON field names are case-sensitive and the external Okta API (and JSON Patch standard, RFC 6902) expects a lowercase 'value', leading to serialization/deserialization failures.
Additional Locations (4)
luisina-santos
force-pushed
the
goldschmidt/upgrade-role-okta-v5-groups
branch
from
93c7aba to
4cc0f88
Compare
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughMigrates Okta integration from v2 to v5 across connectors, pagination, and error handling. Introduces v5-specific pagination utilities, centralized Okta error mapping, and extensive signature updates to use oktav5 types. Updates tests and models accordingly. Also bumps the Okta SDK dependency to v5.0.8-conductorone. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Caller
participant Connector
participant OktaV5 as oktav5.APIClient
participant Pager as paginateV5
Caller->>Connector: List resources (page token?)
alt Initial page
Connector->>Pager: paginateV5(ctx, client, "")
Pager->>OktaV5: Fetch page (Execute)
OktaV5-->>Pager: Data + APIResponse
Pager-->>Connector: Page data + nextPage + annos
else Next page via token
Connector->>Pager: paginateV5(ctx, client, token)
Pager->>Pager: Deserialize previous resp/link
Pager->>OktaV5: Fetch next page (Execute)
OktaV5-->>Pager: Data + APIResponse
Pager-->>Connector: Page data + nextPage + annos
end
Connector-->>Caller: Resources + nextPage + annotations
sequenceDiagram
autonumber
actor Consumer
participant EventLog as EventLog.ListEvents
participant OktaV5 as oktav5.APIClient
Consumer->>EventLog: ListEvents(cursor?)
alt No cursor
EventLog->>OktaV5: ListLogEvents(...).Execute()
else With cursor
EventLog->>EventLog: Deserialize prev response
EventLog->>OktaV5: Next page (Execute)
end
OktaV5-->>EventLog: Events + APIResponse
EventLog->>EventLog: parseRespV5 -> nextCursor, annos
EventLog-->>Consumer: Events, nextCursor, annos
note over EventLog: Errors wrapped via wrapErrorV5
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120+ minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
pkg/connector/event_log.go (1)
115-130: Handleresp == nilbefore callingparseRespV5
When the cursor has no next page,prevAPIResponse.HasNextPage()is false andrespstaysnil, but we still pass it intoparseRespV5. That leads to a nil-pointer panic inside the parser. Please short-circuit in the “no next page” branch (e.g., reuseprevAPIResponseor skip parsing entirely) before callingparseRespV5.pkg/connector/event_filters.go (1)
148-178: Fix pointer usage inRoleMembershipFilter(current build failure)
role.DisplayName,user.DisplayName,roleType.Type, anduser.Idare*string. Passing them straight intosdkResource.NewResource(or formatting strings) breaks the build (cannot use role.DisplayName (*string) as stringper CI) and will panic on nil. Convert them withnullableStrand validate before proceeding.- roleResource, err := sdkResource.NewResource(role.DisplayName, resourceTypeRole, roleType.Type) + roleName := nullableStr(role.DisplayName) + roleTypeID := nullableStr(roleType.Type) + if roleName == "" || roleTypeID == "" { + return fmt.Errorf("okta-connectorv2: missing role display name or type on event target") + } + roleResource, err := sdkResource.NewResource(roleName, resourceTypeRole, roleTypeID) ... - principal, err := sdkResource.NewResource(user.DisplayName, resourceTypeUser, user.Id) + userName := nullableStr(user.DisplayName) + userID := nullableStr(user.Id) + if userName == "" || userID == "" { + return fmt.Errorf("okta-connectorv2: missing user id or display name on event target") + } + principal, err := sdkResource.NewResource(userName, resourceTypeUser, userID) ... - userTrait, err := sdkResource.NewUserTrait(sdkResource.WithEmail(user.AlternateId, true)) + userEmail := nullableStr(user.AlternateId) + userTrait, err := sdkResource.NewUserTrait(sdkResource.WithEmail(userEmail, true))These fixes clear the compilation error and prevent runtime panics.
pkg/connector/resource_sets.go (1)
61-76: Pointer deref without nil checks may panicresourceSetResource dereferences rs.Id/rs.Label/rs.Description directly. These fields are pointers in v5 models and can be nil. Align with resourceSetsResource by using nullableStr to avoid potential panics.
- profile := map[string]interface{}{ - "id": *rs.Id, - "label": *rs.Label, - "description": *rs.Description, - } + profile := map[string]interface{}{ + "id": nullableStr(rs.Id), + "label": nullableStr(rs.Label), + "description": nullableStr(rs.Description), + } @@ - return sdkResource.NewResource( - *rs.Label, + return sdkResource.NewResource( + nullableStr(rs.Label), resourceTypeResourceSets, - *rs.Id, + nullableStr(rs.Id),pkg/connector/resource_sets_bindings.go (1)
258-279: Resetprincipaleach iteration to avoid stale grants.Line 258 declares
principaloutside the loop, but inside the loop we never zero it out before inspecting a member. IfmemberHrefyields an unexpected type,principalkeeps the previous user/group pointer and we end up emitting a grant for the wrong principal. Setprincipal = nil(or redeclare it) at the top of the loop so each member is evaluated independently.Apply this diff:
- for _, member := range members.Members { + for _, member := range members.Members { + principal = nil
♻️ Duplicate comments (11)
pkg/connector/event_filter.go (1)
71-77: Nil derefs and address-of-range-variable bug; fix matching and handlingMultiple unsafe pointer derefs and a Go range-address bug will cause panics and incorrect target maps.
- if !filter.EventTypes.Contains(*event.EventType) { + if event.EventType == nil || !filter.EventTypes.Contains(*event.EventType) { return false } @@ - if filter.ActorType != "" && filter.ActorType != *event.Actor.Type { + if filter.ActorType != "" { + if event.Actor == nil || event.Actor.Type == nil || filter.ActorType != *event.Actor.Type { + return false + } + } return false } @@ - for _, target := range event.Target { - targetSet.Add(*target.Type) - } + for _, target := range event.Target { + if target.Type == nil { + continue + } + targetSet.Add(*target.Type) + } @@ - for _, target := range event.Target { - targetMap[*target.Type] = append(targetMap[*target.Type], &target) - } + for i := range event.Target { + t := &event.Target[i] // pointer to slice element; safe + if t.Type == nil { + continue + } + targetMap[*t.Type] = append(targetMap[*t.Type], t) + } @@ - rv := &v2.Event{ - Id: *event.Uuid, - OccurredAt: timestamppb.New(*event.Published), - } + id := "" + if event.Uuid != nil { + id = *event.Uuid + } + var occurredAt *timestamppb.Timestamp + if event.Published != nil { + occurredAt = timestamppb.New(*event.Published) + } + rv := &v2.Event{ + Id: id, + OccurredAt: occurredAt, + }Also applies to: 83-85, 97-104
pkg/connector/event_filters.go (2)
55-80: Defensive handling for pointer-valued targets
Okta v5 exposesLogTargetfields (Id, DisplayName, etc.) as pointers. Directly dereferencing them (e.g.,*userGroup.DisplayName,*user.Id) will panic when the value is absent—a scenario Okta already hit and was flagged in prior reviews. Please guard each pointer before use, and either skip the event or fall back tonullableStr/default text.- resource, err := sdkResource.NewResource(*userGroup.DisplayName, resourceTypeGroup, *userGroup.Id) + groupID := nullableStr(userGroup.Id) + groupName := nullableStr(userGroup.DisplayName) + if groupID == "" || groupName == "" { + return fmt.Errorf("okta-connectorv2: missing group id or display name on event target") + } + resource, err := sdkResource.NewResource(groupName, resourceTypeGroup, groupID)Apply the same guarding for the user target and all related logging statements in this block.
208-230: Nil-check authentication event fields
event.Actor.AlternateId,event.Actor.Id,event.Actor.DisplayName, andappInstance.DisplayNameare all pointers in v5. Directly dereferencing them (e.g., Lines 208-226) can panic when Okta omits those attributes—previous reviews already called this out. Please usenullableStr(and skip the event if a required identifier is empty) before constructing resources or traits.pkg/connector/event_log.go (1)
99-121: Guard pointer fields before dereferencing
log.EventType,userGroup.DisplayName, and other Okta v5 fields are pointers. Dereferencing them (e.g., Line 101) will panic when Okta omits the value, which was already called out in earlier review feedback. Please gate onnil(or usenullableStr) before using these values, and skip/continue when the identifier you need is missing.- for _, log := range logs { - relevantFilters := filterMap[*log.EventType] + for _, log := range logs { + eventType := nullableStr(log.EventType) + if eventType == "" { + l.Debug("okta-connectorv2: skipping log with empty event type") + continue + } + relevantFilters := filterMap[eventType] for _, filter := range relevantFilters { - if filter.Matches(&log) { + if filter.Matches(&log) { event, err := filter.Handle(l, &log) // MJP we don't want to stop, we should just log the error and continue if err != nil { - l.Error("error handling event", zap.Error(err), zap.String("event_type", *log.EventType)) + l.Error("error handling event", zap.Error(err), zap.String("event_type", eventType)) } else { rv = append(rv, event) } } } }Make the same adjustment for every other dereference in this loop (e.g., building log messages) to avoid runtime panics.
pkg/connector/ciam.go (1)
224-233: Log the actualErrorSummary
The warning path still logsErrorSummaryusingerrOkta.ErrorCode, which is the bug flagged in the earlier review. Please switch toerrOkta.ErrorSummaryso we emit the human-readable message.- zap.String("ErrorSummary", nullableStr(errOkta.ErrorCode)), + zap.String("ErrorSummary", nullableStr(errOkta.ErrorSummary)),pkg/connector/aws_account.go (2)
182-186: Same nil guard issue as abovePlease apply the same
||nil check fix here and in other copies of this pattern so we don’t dereference a nilProfile.
103-108: Fix the nil-profile guard
group.Profile == nil && group.Profile.Name == nilstill dereferencesgroup.Profile.Namewhengroup.Profileis nil, causing a panic. Switch the condition to use||, or split the checks, before touchinggroup.Profile.Name.- if group.Profile == nil && group.Profile.Name == nil { + if group.Profile == nil || group.Profile.Name == nil {pkg/connector/app.go (1)
702-705: Fix nil check before accessingProfile.NameSame issue here: use
||(or separate checks) so we don’t dereferencegroup.Profilewhen it is nil. Please update all occurrences in this function accordingly.pkg/connector/group.go (1)
338-348: Protect against nilProfileSame panic risk as in other files:
group.Profilecan be nil, so this condition dereferencesgroup.Profile.Name. Switch to||(or split checks) before usinggroup.Profile.Name, and apply the same fix anywhere else this pattern appears.pkg/connector/role.go (1)
85-88: Restore JSON tag to lowercasevalueThe JSON Patch standard (RFC 6902) and Okta’s API both expect the payload field to be
"value". Changing the tag to"Value"breaks role-assignment PATCH requests. Please revert the tag tojson:"value,omitempty"(same fix needed in every struct where the tag flipped).pkg/connector/custom_role.go (1)
153-168: Guard against nilrole.Typebefore dereferencing
role.Typestill arrives as a pointer in v5 responses, and Okta omits it for some assignments (e.g., org admins fetched via/api/v1/roles). Dereferencing without checking will panic. Please ensurerole.Type != nilbefore dereferencing—mirroring the existing guards forStatusandAssignmentType.- if *role.Type == roleTypeCustom { + if role.Type == nil { + continue + } + if *role.Type == roleTypeCustom {
🧹 Nitpick comments (6)
pkg/connector/oktaerrors/error.json (1)
105-112: Inconsistent HTTP status for “Unsupported media type”“Unsupported media type” typically maps to 415 (Unsupported Media Type), not 404 (Not Found). Please verify against Okta’s docs/source and correct.
- "statusCode": 404, - "statusReasonPhrase": "Not Found", + "statusCode": 415, + "statusReasonPhrase": "Unsupported Media Type",pkg/connector/api_token.go (1)
48-50: Use the paginator-provided context (ctx2) inside the callbackEnsures cancellations/timeouts/backoff are honored.
- apiTokens, nextPage, annos, err := paginateV5(ctx, o.clientV5, page, func(ctx2 context.Context) ([]oktav5.ApiToken, *oktav5.APIResponse, error) { - return o.clientV5.ApiTokenAPI.ListApiTokens(ctx).Execute() + apiTokens, nextPage, annos, err := paginateV5(ctx, o.clientV5, page, func(ctx2 context.Context) ([]oktav5.ApiToken, *oktav5.APIResponse, error) { + return o.clientV5.ApiTokenAPI.ListApiTokens(ctx2).Execute() })pkg/connector/oktaerrors/errors.go (1)
9-13: Comment nit“Embed into the JS” is misleading in Go code. Consider “Embed error definitions” for clarity.
pkg/connector/pagination.go (1)
54-66: Guard empty NextPage before url.ParseWhen NextPage is empty, url.Parse("") returns a zero URL; current code works but add a fast-path to skip parse for clarity.
- if resp != nil { - u, err := url.Parse(resp.NextPage()) + if resp != nil { + if resp.NextPage() == "" { + // no next page + } else { + u, err := url.Parse(resp.NextPage()) if err != nil { return "", nil, err } - - // Grab entire query param for next Page token, drop limit so we can still set it how we want. - nextQp := u.Query() - nextQp.Del("limit") - nextPage = nextQp.Encode() + // Grab entire query param for next Page token, drop limit so we can still set it how we want. + nextQp := u.Query() + nextQp.Del("limit") + nextPage = nextQp.Encode() + }pkg/connector/resource_sets.go (1)
277-281: Use Info (not Warn) on successful revokeLogging a success (204) as Warn is misleading. Prefer Info or Debug.
- l.Warn("Resource Set Membership has been revoked", + l.Info("Resource Set membership revoked", zap.String("Status", response.Status), )pkg/connector/integration_debug_test.go (1)
200-207: Resource type mismatch in test setupresourceSetsResourceType initialized with resourceTypeUser; use resourceTypeResourceSets for clarity and consistency.
- o := &resourceSetsResourceType{ - resourceType: resourceTypeUser, + o := &resourceSetsResourceType{ + resourceType: resourceTypeResourceSets, clientV5: cliTest.clientV5, }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
go.sumis excluded by!**/*.sumand included by nonevendor/github.com/conductorone/okta-sdk-golang/v5/okta/client.gois excluded by!vendor/**and included by nonevendor/modules.txtis excluded by!vendor/**and included by none
📒 Files selected for processing (23)
go.mod(1 hunks)pkg/connector/api_token.go(4 hunks)pkg/connector/app.go(21 hunks)pkg/connector/aws_account.go(16 hunks)pkg/connector/ciam.go(14 hunks)pkg/connector/connector.go(9 hunks)pkg/connector/custom_role.go(6 hunks)pkg/connector/event_filter.go(3 hunks)pkg/connector/event_filters.go(9 hunks)pkg/connector/event_log.go(2 hunks)pkg/connector/group.go(20 hunks)pkg/connector/helpers.go(2 hunks)pkg/connector/integration_debug_test.go(15 hunks)pkg/connector/models.go(0 hunks)pkg/connector/oktaerrors/error.json(1 hunks)pkg/connector/oktaerrors/errors.go(1 hunks)pkg/connector/oktaerrors/errors_test.go(1 hunks)pkg/connector/pagination.go(3 hunks)pkg/connector/pagination_v5.go(3 hunks)pkg/connector/resource_sets.go(10 hunks)pkg/connector/resource_sets_bindings.go(8 hunks)pkg/connector/role.go(16 hunks)pkg/connector/user.go(19 hunks)
💤 Files with no reviewable changes (1)
- pkg/connector/models.go
🧰 Additional context used
🧬 Code graph analysis (19)
pkg/connector/oktaerrors/errors_test.go (1)
pkg/connector/oktaerrors/errors.go (1)
FindError(50-54)
pkg/connector/event_log.go (2)
pkg/connector/connector.go (2)
Okta(27-43)New(376-485)vendor/github.com/conductorone/baton-sdk/pkg/pagination/pagination.go (1)
StreamState(13-16)
pkg/connector/event_filter.go (1)
vendor/github.com/deckarep/golang-set/v2/set.go (1)
NewSet(201-207)
pkg/connector/resource_sets.go (3)
vendor/github.com/conductorone/okta-sdk-golang/v5/okta/client.go (1)
APIClient(86-248)vendor/github.com/conductorone/baton-sdk/pkg/pagination/pagination.go (1)
Token(18-21)pkg/connector/connector.go (1)
New(376-485)
pkg/connector/custom_role.go (9)
vendor/github.com/conductorone/baton-sdk/pkg/pagination/pagination.go (1)
Token(18-21)vendor/github.com/conductorone/baton-sdk/pb/c1/connector/v2/resource.pb.go (9)
ResourceId(1532-1539)ResourceId(1552-1552)ResourceId(1567-1569)ResourceType(135-145)ResourceType(158-158)ResourceType(173-175)Resource(1592-1604)Resource(1617-1617)Resource(1632-1634)pkg/connector/models.go (2)
Roles(29-41)Role(61-64)vendor/github.com/conductorone/baton-sdk/pkg/types/entitlement/entitlement.go (1)
NewAssignmentEntitlement(60-73)pkg/connector/helpers.go (1)
V1MembershipEntitlementID(36-38)vendor/github.com/conductorone/okta-sdk-golang/v5/okta/client.go (1)
APIClient(86-248)vendor/github.com/conductorone/baton-sdk/pkg/ratelimit/http.go (1)
ExtractRateLimitData(66-130)vendor/github.com/conductorone/baton-sdk/pkg/types/resource/resource.go (1)
NewRoleResource(288-303)vendor/github.com/conductorone/baton-sdk/pkg/types/resource/role_trait.go (2)
RoleTraitOption(11-11)WithRoleProfile(13-24)
pkg/connector/app.go (2)
vendor/github.com/conductorone/okta-sdk-golang/v5/okta/client.go (1)
APIClient(86-248)pkg/connector/connector.go (2)
New(376-485)ResourceNotFoundExceptionErrorCode(23-23)
pkg/connector/helpers.go (4)
vendor/github.com/conductorone/okta-sdk-golang/v5/okta/client.go (2)
GenericOpenAPIError(1571-1575)APIClient(86-248)pkg/connector/oktaerrors/errors.go (1)
FindError(50-54)vendor/github.com/conductorone/baton-sdk/pkg/annotations/annotations.go (1)
Annotations(12-12)pkg/connector/connector.go (1)
New(376-485)
pkg/connector/connector.go (1)
pkg/connector/aws_account.go (1)
OktaAppGroupWrapper(26-28)
pkg/connector/api_token.go (3)
vendor/github.com/conductorone/baton-sdk/pkg/pagination/pagination.go (1)
Token(18-21)vendor/github.com/conductorone/baton-sdk/pb/c1/connector/v2/resource.pb.go (6)
ResourceId(1532-1539)ResourceId(1552-1552)ResourceId(1567-1569)ResourceType(135-145)ResourceType(158-158)ResourceType(173-175)pkg/connector/connector.go (1)
New(376-485)
pkg/connector/aws_account.go (2)
pkg/connector/connector.go (1)
New(376-485)vendor/github.com/conductorone/baton-sdk/pkg/pagination/pagination.go (1)
Token(18-21)
pkg/connector/group.go (5)
vendor/github.com/conductorone/baton-sdk/pkg/pagination/pagination.go (1)
Token(18-21)vendor/github.com/conductorone/baton-sdk/pb/c1/connector/v2/resource.pb.go (9)
ResourceId(1532-1539)ResourceId(1552-1552)ResourceId(1567-1569)ResourceType(135-145)ResourceType(158-158)ResourceType(173-175)Resource(1592-1604)Resource(1617-1617)Resource(1632-1634)pkg/connector/connector.go (1)
New(376-485)vendor/github.com/conductorone/baton-sdk/pb/c1/connector/v2/annotation_etag.pb.go (3)
ETagMetadata(77-82)ETagMetadata(95-95)ETagMetadata(110-112)vendor/github.com/conductorone/okta-sdk-golang/v5/okta/client.go (1)
APIClient(86-248)
pkg/connector/role.go (5)
vendor/github.com/conductorone/okta-sdk-golang/v5/okta/client.go (1)
APIClient(86-248)pkg/connector/connector.go (2)
Okta(27-43)New(376-485)pkg/connector/helpers.go (1)
V1MembershipEntitlementID(36-38)vendor/github.com/conductorone/baton-sdk/pkg/pagination/pagination.go (1)
Token(18-21)vendor/github.com/conductorone/baton-sdk/pkg/types/resource/resource.go (1)
NewRoleResource(288-303)
pkg/connector/user.go (4)
vendor/github.com/conductorone/baton-sdk/pkg/pagination/pagination.go (1)
Token(18-21)pkg/connector/connector.go (1)
New(376-485)vendor/github.com/conductorone/baton-sdk/pkg/types/resource/user_trait.go (2)
UserTraitOption(14-14)WithUserProfile(76-87)pkg/connector/role.go (1)
ContentType(92-92)
pkg/connector/integration_debug_test.go (2)
vendor/github.com/conductorone/okta-sdk-golang/v5/okta/client.go (2)
APIClient(86-248)NewAPIClient(810-921)pkg/connector/connector.go (3)
Okta(27-43)Config(101-122)New(376-485)
pkg/connector/ciam.go (3)
vendor/github.com/conductorone/okta-sdk-golang/v5/okta/client.go (1)
APIClient(86-248)pkg/connector/connector.go (1)
New(376-485)pkg/connector/role.go (1)
ContentType(92-92)
pkg/connector/pagination.go (3)
vendor/github.com/conductorone/baton-sdk/pkg/annotations/annotations.go (1)
Annotations(12-12)vendor/github.com/conductorone/baton-sdk/pkg/ratelimit/http.go (1)
ExtractRateLimitData(66-130)vendor/github.com/conductorone/baton-sdk/pkg/pagination/pagination.go (1)
Bag(31-34)
pkg/connector/pagination_v5.go (3)
pkg/connector/models.go (2)
Link(23-25)Next(19-21)vendor/github.com/conductorone/okta-sdk-golang/v5/okta/client.go (1)
APIClient(86-248)vendor/github.com/conductorone/baton-sdk/pkg/annotations/annotations.go (1)
Annotations(12-12)
pkg/connector/event_filters.go (4)
vendor/github.com/conductorone/baton-sdk/pkg/types/resource/resource.go (1)
NewResource(228-246)pkg/connector/event_filter.go (1)
EventFilter(16-26)pkg/connector/role.go (1)
StandardRoleTypeFromLabel(352-359)vendor/github.com/conductorone/baton-sdk/pkg/types/resource/user_trait.go (2)
NewUserTrait(132-153)WithEmail(32-47)
pkg/connector/resource_sets_bindings.go (5)
pkg/connector/models.go (7)
ResourceSet(66-68)ResourceSets(10-17)Roles(29-41)Role(61-64)Members(48-50)Links(56-59)Self(52-54)vendor/github.com/conductorone/baton-sdk/pkg/types/resource/resource.go (1)
NewResource(228-246)vendor/github.com/conductorone/baton-sdk/pkg/pagination/generic_bag.go (1)
GenBagFromToken(23-34)vendor/github.com/conductorone/okta-sdk-golang/v5/okta/client.go (1)
APIClient(86-248)pkg/connector/connector.go (1)
New(376-485)
🪛 GitHub Actions: ci
pkg/connector/event_filters.go
[error] 155-155: cannot use role.DisplayName (*string) as string value in argument to sdkResource.NewResource. Build step 'go build ./cmd/baton-okta' failed.
🪛 GitHub Check: go-lint
pkg/connector/user.go
[failure] 510-510:
cannot use credentialOptions (variable of type *v2.LocalCredentialOptions) as *v2.CredentialOptions value in argument to getAccountCreationQueryParamsV5 (typecheck)
🪛 GitHub Check: test
pkg/connector/user.go
[failure] 510-510:
cannot use credentialOptions (variable of type *v2.LocalCredentialOptions) as *v2.CredentialOptions value in argument to getAccountCreationQueryParamsV5
🔇 Additional comments (3)
go.mod (1)
7-7: Confirmed okta-sdk-golang/v5 v5.0.8-conductorone is published and listed in go.modpkg/connector/pagination_v5.go (1)
121-147: No-op path returns zero T without error; confirm call-sites tolerate empty pageIf previous.HasNextPage() is false, response remains zero-value and resp is nil. parseRespV5 will return empty next token and nil annos. Ensure callers handle zero T appropriately when no further pages exist.
pkg/connector/app.go (1)
345-349: Nil-profile guard still unsafeThis repeats the same panic risk:
group.Profilecan be nil, so accessinggroup.Profile.Nameinside an&&condition still dereferences a nil pointer. Please make it||(and apply the same fix everywhere this pattern appears in the file, e.g.,getAWSGroupV5).- if group.Profile == nil && group.Profile.Name == nil { + if group.Profile == nil || group.Profile.Name == nil {Likely an incorrect or invalid review comment.
| bag, page, err := parsePageToken(token.Token, &v2.ResourceId{ResourceType: resourceTypeApiToken.Id}) | ||
| if err != nil { | ||
| return nil, "", nil, fmt.Errorf("okta-connector-v5: failed to parse page token: %w", err) | ||
| } | ||
|
|
||
| var apiTokens []oktav5.ApiToken | ||
| var resp *oktav5.APIResponse | ||
|
|
||
| if prevSerializedResp == "" { | ||
| apiTokens, resp, err = o.clientV5.ApiTokenAPI.ListApiTokens(ctx).Execute() | ||
| if err != nil { | ||
| return nil, "", nil, fmt.Errorf("okta-connector-v5: failed to list api tokens: %w", err) | ||
| } | ||
| } else { | ||
| prevResp, err := deserializeOktaResponseV5(prevSerializedResp) | ||
| if err != nil { | ||
| return nil, "", nil, fmt.Errorf("okta-connector-v5: failed to deserialize page token: %w", err) | ||
| } | ||
|
|
||
| localOktaAPIResponse := oktav5.NewAPIResponse(prevResp.Response, o.clientV5, nil) | ||
| if localOktaAPIResponse.HasNextPage() { | ||
| resp, err = localOktaAPIResponse.Next(&apiTokens) | ||
| if err != nil { | ||
| return nil, "", nil, err | ||
| } | ||
| } | ||
| return nil, "", nil, fmt.Errorf("okta-connector-v5: failed to parse Page token: %w", err) | ||
| } |
There was a problem hiding this comment.
Guard nil page token to avoid panic
token may be nil on first page. Add a safe default.
- bag, page, err := parsePageToken(token.Token, &v2.ResourceId{ResourceType: resourceTypeApiToken.Id})
+ tok := ""
+ if token != nil {
+ tok = token.Token
+ }
+ bag, page, err := parsePageToken(tok, &v2.ResourceId{ResourceType: resourceTypeApiToken.Id})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| bag, page, err := parsePageToken(token.Token, &v2.ResourceId{ResourceType: resourceTypeApiToken.Id}) | |
| if err != nil { | |
| return nil, "", nil, fmt.Errorf("okta-connector-v5: failed to parse page token: %w", err) | |
| } | |
| var apiTokens []oktav5.ApiToken | |
| var resp *oktav5.APIResponse | |
| if prevSerializedResp == "" { | |
| apiTokens, resp, err = o.clientV5.ApiTokenAPI.ListApiTokens(ctx).Execute() | |
| if err != nil { | |
| return nil, "", nil, fmt.Errorf("okta-connector-v5: failed to list api tokens: %w", err) | |
| } | |
| } else { | |
| prevResp, err := deserializeOktaResponseV5(prevSerializedResp) | |
| if err != nil { | |
| return nil, "", nil, fmt.Errorf("okta-connector-v5: failed to deserialize page token: %w", err) | |
| } | |
| localOktaAPIResponse := oktav5.NewAPIResponse(prevResp.Response, o.clientV5, nil) | |
| if localOktaAPIResponse.HasNextPage() { | |
| resp, err = localOktaAPIResponse.Next(&apiTokens) | |
| if err != nil { | |
| return nil, "", nil, err | |
| } | |
| } | |
| return nil, "", nil, fmt.Errorf("okta-connector-v5: failed to parse Page token: %w", err) | |
| } | |
| tok := "" | |
| if token != nil { | |
| tok = token.Token | |
| } | |
| bag, page, err := parsePageToken(tok, &v2.ResourceId{ResourceType: resourceTypeApiToken.Id}) | |
| if err != nil { | |
| return nil, "", nil, fmt.Errorf("okta-connector-v5: failed to parse Page token: %w", err) | |
| } |
🤖 Prompt for AI Agents
In pkg/connector/api_token.go around lines 43-46, the code calls
parsePageToken(token.Token, ...) but token can be nil on the first page, which
causes a panic; add a nil-check before calling parsePageToken and pass a safe
default (e.g., an empty string) when token is nil so parsePageToken is invoked
with a valid string, then proceed to handle bag, page, err as before.
| rs, err := apiTokenResource(apiToken) | ||
| if err != nil { | ||
| return nil, annos, err | ||
| } | ||
|
|
||
| return resource, annos, nil | ||
| return rs, annos, nil | ||
| } |
There was a problem hiding this comment.
Harden apiTokenResource against nil pointer fields
apiTokenResource dereferences several pointer fields (ExpiresAt, UserId, LastUpdated, Created, Id). If the API ever omits a field, this panics. Add nil checks/defaults in apiTokenResource.
Outside this hunk, update apiTokenResource:
// pkg/connector/api_token.go
func apiTokenResource(apiToken *oktav5.ApiToken) (*v2.Resource, error) {
// Guard required fields
if apiToken == nil || apiToken.Id == nil || apiToken.Name == "" {
return nil, fmt.Errorf("okta-connector-v5: invalid api token payload")
}
// Optional timestamps and user IDs
var (
expiresAt time.Time
lastUsedAt time.Time
createdAt time.Time
userID string
)
if apiToken.ExpiresAt != nil {
expiresAt = *apiToken.ExpiresAt
}
if apiToken.LastUpdated != nil {
lastUsedAt = *apiToken.LastUpdated
}
if apiToken.Created != nil {
createdAt = *apiToken.Created
}
if apiToken.UserId != nil {
userID = *apiToken.UserId
}
opts := []resource.SecretTraitOption{
resource.WithSecretExpiresAt(expiresAt),
resource.WithSecretLastUsedAt(lastUsedAt),
resource.WithSecretCreatedAt(createdAt),
}
if userID != "" {
opts = append(opts,
resource.WithSecretIdentityID(&v2.ResourceId{ResourceType: resourceTypeUser.Id, Resource: userID}),
resource.WithSecretCreatedByID(&v2.ResourceId{ResourceType: resourceTypeUser.Id, Resource: userID}),
)
}
return resource.NewSecretResource(apiToken.Name, resourceTypeApiToken, *apiToken.Id, opts)
}🤖 Prompt for AI Agents
In pkg/connector/api_token.go around lines 104 to 110, apiTokenResource
currently dereferences pointer fields (ExpiresAt, UserId, LastUpdated, Created,
Id) and can panic if any are nil; update the function to validate required
fields (ensure apiToken != nil, apiToken.Id != nil, and apiToken.Name != ""),
then initialize local zero-value defaults for optional fields (time.Time for
timestamps, empty string for userID) and only assign from pointers when they are
non-nil; build the resource options using those safe values, append
identity-related options only if userID is non-empty, and finally call
resource.NewSecretResource with the safe, validated values.
| if err != nil { | ||
| defer response.Body.Close() | ||
| errOkta, err := getError(response) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| if errOkta.ErrorCode != ResourceNotFoundExceptionErrorCode { | ||
| l.Warn( | ||
| "okta-connector: ", | ||
| zap.String("principal_id", principal.Id.String()), | ||
| zap.String("principal_type", principal.Id.ResourceType), | ||
| zap.String("ErrorCode", errOkta.ErrorCode), | ||
| zap.String("ErrorSummary", errOkta.ErrorSummary), | ||
| ) | ||
|
|
||
| return nil, fmt.Errorf("okta-connector: %v", errOkta) | ||
| if errOkta, ok := asErrorV5(err); ok { | ||
| if nullableStr(errOkta.ErrorCode) != ResourceNotFoundExceptionErrorCode { | ||
| l.Warn( | ||
| "okta-connector: ", | ||
| zap.String("principal_id", principal.Id.String()), | ||
| zap.String("principal_type", principal.Id.ResourceType), | ||
| zap.String("ErrorCode", nullableStr(errOkta.ErrorCode)), | ||
| zap.String("ErrorSummary", nullableStr(errOkta.ErrorSummary)), | ||
| ) | ||
| } | ||
|
|
||
| anno, err := wrapErrorV5(response, err, errors.New("okta-connector: GetApplicationGroupAssignment error")) | ||
| return anno, err | ||
| } else { | ||
| anno, err := wrapErrorV5(response, err, errors.New("okta-connector: GetApplicationGroupAssignment error")) | ||
| return anno, err | ||
| } |
There was a problem hiding this comment.
Handle 404 correctly when checking existing group assignment
GetApplicationGroupAssignment returns a 404 when the group hasn’t been assigned yet. The new code wraps that into an error and returns early, so you can never create a fresh group assignment. Mirror the user path: allow E0000007 (404) to pass through without returning, and only wrap non-404 errors.
- if err != nil {
- if errOkta, ok := asErrorV5(err); ok {
- if nullableStr(errOkta.ErrorCode) != ResourceNotFoundExceptionErrorCode {
- ...
- }
-
- anno, err := wrapErrorV5(response, err, errors.New("okta-connector: GetApplicationGroupAssignment error"))
- return anno, err
- } else {
- anno, err := wrapErrorV5(response, err, errors.New("okta-connector: GetApplicationGroupAssignment error"))
- return anno, err
- }
- }
+ if err != nil {
+ if errOkta, ok := asErrorV5(err); ok && nullableStr(errOkta.ErrorCode) == ResourceNotFoundExceptionErrorCode {
+ // Expected when the group has not been assigned yet; continue.
+ } else {
+ anno, err := wrapErrorV5(response, err, errors.New("okta-connector: GetApplicationGroupAssignment error"))
+ return anno, err
+ }
+ }🤖 Prompt for AI Agents
In pkg/connector/app.go around lines 488-505 the code currently wraps and
returns for any error from GetApplicationGroupAssignment, which incorrectly
treats Okta's 404 (E0000007 / ResourceNotFoundExceptionErrorCode) as fatal;
change the control flow so that when asErrorV5(err) is true and
nullableStr(errOkta.ErrorCode) == ResourceNotFoundExceptionErrorCode you do NOT
wrap/return (log or ignore and continue the normal creation path), and only call
wrapErrorV5 and return for non-404 errors; keep the existing logging for non-404
error cases.
| var rv []*v2.Grant | ||
|
|
||
| qp := queryParams(token.Size, page) | ||
| appUsers, respContext, err := listApplicationUsers(ctx, o.connector.client, o.connector.awsConfig.OktaAppId, token, qp) | ||
| appUsers, respContext, err := listApplicationUsersV5(ctx, o.connector.clientV5, o.connector.awsConfig.OktaAppId, page) | ||
| if err != nil { | ||
| return nil, nil, fmt.Errorf("okta-aws-connector: error listing application users %w", err) | ||
| return nil, respContext, fmt.Errorf("okta-aws-connector: error listing application users %w", err) | ||
| } | ||
|
|
There was a problem hiding this comment.
Resolve compile error: unused token parameter
listGroupsHelper now accepts token *pagination.Token but never uses it, so the file fails to compile (token declared but not used). Either drop the parameter or reference it (e.g., _ = token) to keep the signature intact.
🤖 Prompt for AI Agents
In pkg/connector/aws_account.go around lines 304 to 310, the new
listGroupsHelper signature includes a token *pagination.Token parameter that is
never referenced, causing a compile error; fix by either removing the token
parameter from the function signature and all its call sites, or keep the
signature and explicitly reference it (for example add a no-op usage like _ =
token at the top of the function) so the parameter is considered used.
| func (o *groupResourceType) listGroupUsersV5(ctx context.Context, groupID string, token *pagination.Token, after string) ([]oktav5.GroupMember, *oktav5.APIResponse, error) { | ||
| users, resp, err := o.connector.clientV5.GroupAPI.ListGroupUsers(ctx, groupID). | ||
| After(after). | ||
| Limit(defaultLimit). | ||
| Execute() | ||
| if err != nil { | ||
| return nil, nil, err | ||
| return nil, resp, fmt.Errorf("okta-connectorv2: failed to fetch group users from okta: %w", err) | ||
| } |
There was a problem hiding this comment.
Either use or drop the unused token parameter
listGroupUsersV5 still takes token but never references it, so the compiler will complain (token declared but not used). Either remove the parameter or add _ = token to keep the signature without breaking the build.
🤖 Prompt for AI Agents
In pkg/connector/group.go around lines 432 to 439, the function listGroupUsersV5
declares a parameter named token that is never used which causes a compiler
error; either remove the token parameter from the function signature (and update
all call sites) or explicitly mark it unused by adding a single assignment `_ =
token` at the top of the function; choose removal if callers can be updated
easily, otherwise use the `_ = token` placeholder to preserve the signature.
| func TestFindError(t *testing.T) { | ||
| err := FindError("E0000001") | ||
| require.Equal(t, err.ErrorCode, "E0000001") | ||
| } |
There was a problem hiding this comment.
Add nil check and fix Equal argument order
Avoid potential panic and follow testify convention.
func TestFindError(t *testing.T) {
- err := FindError("E0000001")
- require.Equal(t, err.ErrorCode, "E0000001")
+ err := FindError("E0000001")
+ require.NotNil(t, err)
+ require.Equal(t, "E0000001", err.ErrorCode)
}🤖 Prompt for AI Agents
In pkg/connector/oktaerrors/errors_test.go around lines 9 to 12, the test calls
err.ErrorCode without checking err for nil and also uses require.Equal with
arguments reversed; add a require.NotNil(t, err) before accessing err to avoid a
panic, and change require.Equal to use the expected value first
(require.Equal(t, "E0000001", err.ErrorCode)).
| data, err := embeddedErrors.ReadFile("error.json") | ||
| if err != nil { | ||
| panic(err) | ||
| } | ||
|
|
There was a problem hiding this comment.
Avoid panicking on init errors; return empty map and proceed
Panics here will crash the process if the embedded file is missing/corrupt. Prefer returning an empty map (and optionally logging) to preserve connector resilience.
- data, err := embeddedErrors.ReadFile("error.json")
- if err != nil {
- panic(err)
- }
+ data, err := embeddedErrors.ReadFile("error.json")
+ if err != nil {
+ return map[string]*OktaError{}
+ }
@@
- err = json.Unmarshal(data, &errors)
- if err != nil {
- panic(err)
- }
+ err = json.Unmarshal(data, &errors)
+ if err != nil {
+ return map[string]*OktaError{}
+ }Also applies to: 27-30
🤖 Prompt for AI Agents
In pkg/connector/oktaerrors/errors.go around lines 16-20 (and likewise lines
27-30), the init path panics when embeddedErrors.ReadFile or json.Unmarshal
fail; change this to handle errors gracefully by catching the error, optionally
logging it, and returning an empty map (or setting a package-level empty map)
instead of panicking so the connector can continue running; make sure any
function signature or package initialization logic returns/sets an empty map on
error and does not propagate a panic.
| mappedErrors := make(map[string]*OktaError) | ||
| for _, e := range errors.Errors { | ||
| mappedErrors[e.ErrorCode] = &e | ||
| } |
There was a problem hiding this comment.
Bug: address taken of range variable; all entries point to the same element
Taking &e in a range loop stores the address of the loop variable, causing every map value to reference the last item. Use the slice index or create a new variable per iteration.
Apply one of these diffs:
Option A (index):
- for _, e := range errors.Errors {
- mappedErrors[e.ErrorCode] = &e
- }
+ for i := range errors.Errors {
+ e := &errors.Errors[i]
+ mappedErrors[e.ErrorCode] = e
+ }Option B (shadow copy):
- for _, e := range errors.Errors {
- mappedErrors[e.ErrorCode] = &e
- }
+ for _, e := range errors.Errors {
+ e := e
+ mappedErrors[e.ErrorCode] = &e
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| mappedErrors := make(map[string]*OktaError) | |
| for _, e := range errors.Errors { | |
| mappedErrors[e.ErrorCode] = &e | |
| } | |
| mappedErrors := make(map[string]*OktaError) | |
| for i := range errors.Errors { | |
| e := &errors.Errors[i] | |
| mappedErrors[e.ErrorCode] = e | |
| } |
🤖 Prompt for AI Agents
In pkg/connector/oktaerrors/errors.go around lines 32 to 35, the loop takes the
address of the range variable causing every map entry to point to the same
element; fix by either using the slice index to take the address of the actual
element (mappedErrors[errors.Errors[i].ErrorCode] = &errors.Errors[i]) or by
creating a new per-iteration variable to capture the value before taking its
address (v := e; mappedErrors[v.ErrorCode] = &v).
| rawURL, _ := url.Parse(rawLink) | ||
| rawURL.Scheme = "" | ||
| rawURL.Host = "" | ||
| if r.Request != nil { | ||
| q := r.Request.URL.Query() | ||
| for k, v := range rawURL.Query() { | ||
| q.Set(k, v[0]) | ||
| } | ||
| rawURL.RawQuery = q.Encode() | ||
| } | ||
| if strings.Contains(link, `rel="next"`) { | ||
| next = rawURL.Query().Get("after") | ||
| } | ||
| } |
There was a problem hiding this comment.
Handle URL parse errors and empty query values
Avoid ignoring parse errors and unsafe v[0] indexing for query values.
- rawURL, _ := url.Parse(rawLink)
+ rawURL, err := url.Parse(rawLink)
+ if err != nil {
+ continue
+ }
@@
- for k, v := range rawURL.Query() {
- q.Set(k, v[0])
- }
+ for k, v := range rawURL.Query() {
+ if len(v) > 0 {
+ q.Set(k, v[0])
+ }
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| rawURL, _ := url.Parse(rawLink) | |
| rawURL.Scheme = "" | |
| rawURL.Host = "" | |
| if r.Request != nil { | |
| q := r.Request.URL.Query() | |
| for k, v := range rawURL.Query() { | |
| q.Set(k, v[0]) | |
| } | |
| rawURL.RawQuery = q.Encode() | |
| } | |
| if strings.Contains(link, `rel="next"`) { | |
| next = rawURL.Query().Get("after") | |
| } | |
| } | |
| rawURL, err := url.Parse(rawLink) | |
| if err != nil { | |
| continue | |
| } | |
| rawURL.Scheme = "" | |
| rawURL.Host = "" | |
| if r.Request != nil { | |
| q := r.Request.URL.Query() | |
| for k, v := range rawURL.Query() { | |
| if len(v) > 0 { | |
| q.Set(k, v[0]) | |
| } | |
| } | |
| rawURL.RawQuery = q.Encode() | |
| } | |
| if strings.Contains(link, `rel="next"`) { | |
| next = rawURL.Query().Get("after") | |
| } | |
| } |
| func getAccountCreationQueryParamsV5( | ||
| accountInfo *v2.AccountInfo, | ||
| credentialOptions *v2.CredentialOptions, | ||
| ) (bool, string, error) { | ||
| if credentialOptions.GetNoPassword() != nil { | ||
| return nil, nil | ||
| return false, "", nil | ||
| } | ||
|
|
||
| pMap := accountInfo.Profile.AsMap() | ||
| requirePass := pMap["password_change_on_login_required"] | ||
| requirePasswordChanged := false | ||
|
|
||
| switch v := requirePass.(type) { | ||
| case bool: | ||
| requirePasswordChanged = v | ||
| case string: | ||
|
|
||
| parsed, err := strconv.ParseBool(v) | ||
| if err != nil { | ||
| return nil, err | ||
| return false, "", err | ||
| } | ||
| requirePasswordChanged = parsed | ||
| case nil: | ||
| // Do nothing | ||
| } | ||
|
|
||
| params := &query.Params{} | ||
| if requirePasswordChanged { | ||
| params.NextLogin = "changePassword" | ||
| params.Activate = ToPtr(true) // This defaults to true anyways, but lets be explicit | ||
| nextLogin := "changePassword" | ||
|
|
||
| // This defaults to true anyways, but lets be explicit | ||
| return true, nextLogin, nil | ||
| } | ||
| return params, nil | ||
|
|
||
| return false, "", nil | ||
| } |
There was a problem hiding this comment.
Fix signature mismatch breaking the build.
getAccountCreationQueryParamsV5 is declared to take *v2.CredentialOptions, but every caller—including Line 510—passes *v2.LocalCredentialOptions. This is currently a hard compile failure (cannot use credentialOptions … as *v2.CredentialOptions). Change the helper to accept the same type the callers provide (or update the call sites), otherwise the PR will not build.
Apply this diff:
-func getAccountCreationQueryParamsV5(
- accountInfo *v2.AccountInfo,
- credentialOptions *v2.CredentialOptions,
-) (bool, string, error) {
+func getAccountCreationQueryParamsV5(
+ accountInfo *v2.AccountInfo,
+ credentialOptions *v2.LocalCredentialOptions,
+) (bool, string, error) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func getAccountCreationQueryParamsV5( | |
| accountInfo *v2.AccountInfo, | |
| credentialOptions *v2.CredentialOptions, | |
| ) (bool, string, error) { | |
| if credentialOptions.GetNoPassword() != nil { | |
| return nil, nil | |
| return false, "", nil | |
| } | |
| pMap := accountInfo.Profile.AsMap() | |
| requirePass := pMap["password_change_on_login_required"] | |
| requirePasswordChanged := false | |
| switch v := requirePass.(type) { | |
| case bool: | |
| requirePasswordChanged = v | |
| case string: | |
| parsed, err := strconv.ParseBool(v) | |
| if err != nil { | |
| return nil, err | |
| return false, "", err | |
| } | |
| requirePasswordChanged = parsed | |
| case nil: | |
| // Do nothing | |
| } | |
| params := &query.Params{} | |
| if requirePasswordChanged { | |
| params.NextLogin = "changePassword" | |
| params.Activate = ToPtr(true) // This defaults to true anyways, but lets be explicit | |
| nextLogin := "changePassword" | |
| // This defaults to true anyways, but lets be explicit | |
| return true, nextLogin, nil | |
| } | |
| return params, nil | |
| return false, "", nil | |
| } | |
| func getAccountCreationQueryParamsV5( | |
| accountInfo *v2.AccountInfo, | |
| credentialOptions *v2.LocalCredentialOptions, | |
| ) (bool, string, error) { | |
| if credentialOptions.GetNoPassword() != nil { | |
| return false, "", nil | |
| } | |
| pMap := accountInfo.Profile.AsMap() | |
| requirePass := pMap["password_change_on_login_required"] | |
| requirePasswordChanged := false | |
| switch v := requirePass.(type) { | |
| case bool: | |
| requirePasswordChanged = v | |
| case string: | |
| parsed, err := strconv.ParseBool(v) | |
| if err != nil { | |
| return false, "", err | |
| } | |
| requirePasswordChanged = parsed | |
| case nil: | |
| // Do nothing | |
| } | |
| if requirePasswordChanged { | |
| nextLogin := "changePassword" | |
| // This defaults to true anyways, but let's be explicit | |
| return true, nextLogin, nil | |
| } | |
| return false, "", nil | |
| } |
🤖 Prompt for AI Agents
In pkg/connector/user.go around lines 626 to 660, the helper
getAccountCreationQueryParamsV5 currently accepts *v2.CredentialOptions but all
callers pass *v2.LocalCredentialOptions causing a compile error; change the
function signature to accept *v2.LocalCredentialOptions (and update any
parameter name usages if necessary), keep the internal logic the same (e.g.,
calling GetNoPassword()), and run a build to ensure all call sites now match the
updated signature.
1 participant
Upgrade to okta-sdk-v5
Summary by CodeRabbit