Fix 16 bugs across provisioning, sync, and panic paths

pkg/connector/api_token.go

@@ -128,27 +128,42 @@ func (o *apiTokenResourceType) Get(ctx context.Context, resourceId *v2.ResourceI
 }
 
 func apiTokenResource(apiToken *oktav5.ApiToken) (*v2.Resource, error) {
+	tokenId, ok := apiToken.GetIdOk()
+	if !ok {
+		return nil, fmt.Errorf("okta-connectorv2: api token %q has no id", apiToken.GetName())
+	}
+
 	options := []resource.SecretTraitOption{
-		resource.WithSecretExpiresAt(*apiToken.ExpiresAt),
-		resource.WithSecretIdentityID(&v2.ResourceId{
-			ResourceType:  resourceTypeUser.Id,
-			Resource:      *apiToken.UserId,
-			BatonResource: false,
-		}),
-		resource.WithSecretCreatedByID(&v2.ResourceId{
-			ResourceType:  resourceTypeUser.Id,
-			Resource:      *apiToken.UserId,
-			BatonResource: false,
-		}),
-		resource.WithSecretLastUsedAt(*apiToken.LastUpdated),
-		resource.WithSecretCreatedAt(*apiToken.Created),
 		resource.WithSecretType(v2.SecretTrait_CREDENTIAL_TYPE_STATIC_SECRET),
 		resource.WithSecretDetail("okta.api_token"),
 	}
+	if expiresAt, ok := apiToken.GetExpiresAtOk(); ok {
+		options = append(options, resource.WithSecretExpiresAt(*expiresAt))
+	}
+	if userId, ok := apiToken.GetUserIdOk(); ok {
+		options = append(options,
+			resource.WithSecretIdentityID(&v2.ResourceId{
+				ResourceType:  resourceTypeUser.Id,
+				Resource:      *userId,
+				BatonResource: false,
+			}),
+			resource.WithSecretCreatedByID(&v2.ResourceId{
+				ResourceType:  resourceTypeUser.Id,
+				Resource:      *userId,
+				BatonResource: false,
+			}),
+		)
+	}
+	if lastUpdated, ok := apiToken.GetLastUpdatedOk(); ok {
+		options = append(options, resource.WithSecretLastUsedAt(*lastUpdated))
+	}
+	if created, ok := apiToken.GetCreatedOk(); ok {
+		options = append(options, resource.WithSecretCreatedAt(*created))
+	}
 	rv, err := resource.NewSecretResource(
 		apiToken.Name,
 		resourceTypeApiToken,
-		*apiToken.Id,
+		*tokenId,
 		options,
 	)
 	if err != nil {

pkg/connector/app.go

@@ -151,7 +151,7 @@ func (o *appResourceType) Grants(
 			return nil, nil, fmt.Errorf("okta-connectorv2: failed to list app users grants: %w", err)
 		}
 	default:
-		return nil, nil, fmt.Errorf("okta-connectorv2: unexpected resource for app: %w", err)
+		return nil, nil, fmt.Errorf("okta-connectorv2: unexpected resource for app: %s", bag.ResourceID())
 	}
 
 	pageToken, err := bag.Marshal()
@@ -533,8 +533,16 @@ func (g *appResourceType) Revoke(ctx context.Context, grant *v2.Grant) (annotati
 		}
 	case resourceTypeGroup.Id:
 		groupID := principal.Id.Resource
-		_, _, err := g.client.Application.GetApplicationGroupAssignment(ctx, appID, groupID, nil)
+		_, resp, err := g.client.Application.GetApplicationGroupAssignment(ctx, appID, groupID, nil)
 		if err != nil {
+			if resp != nil && resp.StatusCode == http.StatusNotFound {
+				l.Debug(
+					"okta-connector: revoke: group does not have app membership",
+					zap.String("principal_id", principal.Id.String()),
+					zap.String("principal_type", principal.Id.ResourceType),
+				)
+				return annotations.New(&v2.GrantAlreadyRevoked{}), nil
+			}
 			l.Warn(
 				"okta-connector: group does not have app membership",
 				zap.String("principal_id", principal.Id.String()),

pkg/connector/connector.go

@@ -187,7 +187,7 @@ func (c *Okta) Metadata(ctx context.Context) (*v2.ConnectorMetadata, error) {
 
 	var annos annotations.Annotations
 	annos.Update(&v2.ExternalLink{
-		Url: c.domain,
+		Url: fmt.Sprintf("https://%s", c.domain),
 	})
 
 	return &v2.ConnectorMetadata{

pkg/connector/custom_role.go

@@ -159,10 +159,25 @@ func (o *customRoleResourceType) Grants(
 		return nil, nil, fmt.Errorf("okta-connectorv2: failed to fetch bag.Next: %w", err)
 	}
 
-	// Collect all user IDs upfront, so that we can get all their roles from cache at once.
+	// Filter users by email domain first to avoid fetching roles for users we'll skip anyway.
 	userIDs := make([]string, 0, len(usersWithRoleAssignments))
 	for _, user := range usersWithRoleAssignments {
-		userIDs = append(userIDs, user.Id)
+		userId := user.Id
+
+		// Check if the user should be included after filtering by email domains.
+		shouldInclude, ok := o.connector.shouldIncludeUserFromCache(ctx, attrs.Session, userId)
+		if !ok {
+			user, _, err := o.connector.client.User.GetUser(ctx, userId)
+			if err != nil {
+				return nil, nil, err
+			}
+			shouldInclude = o.connector.shouldIncludeUserAndSetCache(ctx, attrs.Session, user)
+		}
+		if !shouldInclude {
+			continue
+		}
+
+		userIDs = append(userIDs, userId)
 	}
 
 	// Get all cached roles at once (this will only return roles that were found).
@@ -176,9 +191,7 @@ func (o *customRoleResourceType) Grants(
 	// Now, get any remaining user roles (those that aren't in cache)
 	// and keep track of them so that we can try to cache them later.
 	toCache := make(map[string]mapset.Set[string])
-	for _, user := range usersWithRoleAssignments {
-		userId := user.Id
-
+	for _, userId := range userIDs {
 		// If this user's roles are already cached, keep moving.
 		if _, found := userRoles[userId]; found {
 			continue

pkg/connector/event_filter.go

@@ -98,6 +98,10 @@ func (filter *EventFilter) Handle(l *zap.Logger, event *oktaSDK.LogEvent) (*v2.E
 		targetMap[target.Type] = append(targetMap[target.Type], target)
 	}
 
+	if event.Published == nil {
+		return nil, fmt.Errorf("okta-connectorv2: event %s has no published timestamp", event.Uuid)
+	}
+
 	rv := &v2.Event{
 		Id:         event.Uuid,
 		OccurredAt: timestamppb.New(*event.Published),

pkg/connector/event_filters.go

@@ -132,7 +132,7 @@ var (
 		},
 	}
 	ApplicationLifecycleFilter = EventFilter{
-		EventTypes:  mapset.NewSet[string]("app.lifecycle.create", "application.lifecycle.update"),
+		EventTypes:  mapset.NewSet[string]("application.lifecycle.create", "application.lifecycle.update"),
 		TargetTypes: mapset.NewSet[string]("AppInstance"),
 		EventHandler: func(l *zap.Logger, event *oktaSDK.LogEvent, targetMap map[string][]*oktaSDK.LogTarget, rv *v2.Event) error {
 			if len(targetMap["AppInstance"]) != 1 {

pkg/connector/group.go

@@ -198,13 +198,6 @@ func (o *groupResourceType) Grants(
 			return nil, nil, err
 		}
 
-		if pageToken == "" {
-			etag := &v2.ETag{
-				Value: time.Now().UTC().Format(time.RFC3339Nano),
-			}
-			annos.Update(etag)
-		}
-
 		return rv, &sdkResource.SyncOpResults{NextPageToken: pageToken, Annotations: annos}, nil
 	case resourceTypeRole.Id:
 		roles, resp, err := listGroupAssignedRoles(ctx, o.connector.client, groupID, nil)
@@ -293,6 +286,13 @@ func (o *groupResourceType) Grants(
 			return nil, nil, err
 		}
 
+		if pageToken == "" {
+			etag := &v2.ETag{
+				Value: time.Now().UTC().Format(time.RFC3339Nano),
+			}
+			annos.Update(etag)
+		}
+
 		return rv, &sdkResource.SyncOpResults{NextPageToken: pageToken, Annotations: annos}, nil
 	default:
 		return nil, nil, fmt.Errorf("okta-connector: invalid grant resource type: %s", bag.ResourceTypeID())
@@ -376,7 +376,9 @@ func (o *groupResourceType) groupResource(ctx context.Context, group *okta.Group
 	if err != nil {
 		return nil, err
 	}
-	annos.Update(etagMd)
+	if etagMd != nil {
+		annos.Update(etagMd)
+	}
 
 	if group.Type == builtInGroupType {
 		annos.Update(&v2.EntitlementImmutable{})
@@ -607,7 +609,7 @@ func (o *groupResourceType) registerModifyGroupAction(ctx context.Context, regis
 				DisplayName: "Description",
 				Description: "The new description for the group.",
 				Field:       &config.Field_StringField{},
-				IsRequired: false,
+				IsRequired:  false,
 			},
 		},
 		ReturnTypes: []*config.Field{

pkg/connector/resource_sets.go

@@ -60,15 +60,15 @@ func resourceSetsResource(ctx context.Context, rs *ResourceSets, parentResourceI
 
 func resourceSetResource(ctx context.Context, rs *oktav5.ResourceSet, parentResourceID *v2.ResourceId) (*v2.Resource, error) {
 	profile := map[string]interface{}{
-		"id":                    *rs.Id,
-		profileFieldLabel:       *rs.Label,
-		profileFieldDescription: *rs.Description,
+		"id":                    rs.GetId(),
+		profileFieldLabel:       rs.GetLabel(),
+		profileFieldDescription: rs.GetDescription(),
 	}
 
 	return sdkResource.NewResource(
-		*rs.Label,
+		rs.GetLabel(),
 		resourceTypeResourceSets,
-		*rs.Id,
+		rs.GetId(),
 		sdkResource.WithParentResourceID(parentResourceID),
 		sdkResource.WithAppTrait(
 			sdkResource.WithAppProfile(profile),
@@ -157,7 +157,7 @@ func (rs *resourceSetsResourceType) Entitlements(_ context.Context, resource *v2
 			sdkEntitlement.WithAnnotation(&v2.V1Identifier{
 				Id: V1MembershipEntitlementID(resource.Id.GetResource()),
 			}),
-			sdkEntitlement.WithGrantableTo(resourceTypeResourceSets),
+			sdkEntitlement.WithGrantableTo(resourceTypeCustomRole),
 			sdkEntitlement.WithDisplayName(fmt.Sprintf("%s Resource Set Binding", resource.DisplayName)),
 			sdkEntitlement.WithDescription(fmt.Sprintf("Member of %s resource-set in Okta", resource.DisplayName)),
 		),
@@ -170,13 +170,17 @@ func listBindings(
 	ctx context.Context,
 	client *okta.Client,
 	resourceSetId string,
-	_ *query.Params,
+	qp *query.Params,
 ) ([]Role, *okta.Response, error) {
 	apiPath, err := url.JoinPath(apiPathListIamResourceSets, resourceSetId, "bindings")
 	if err != nil {
 		return nil, nil, err
 	}
 
+	if qp != nil {
+		apiPath += qp.String()
+	}
+
 	var resourceSetsBindings *ResourceSetsBindingsAPIData
 	resp, err := doRequest(ctx, apiPath, http.MethodGet, &resourceSetsBindings, client)
 	if err != nil {
@@ -243,7 +247,7 @@ func (rs *resourceSetsResourceType) Grants(ctx context.Context, resource *v2.Res
 		principal := &v2.Resource{Id: &v2.ResourceId{ResourceType: resourceTypeCustomRole.Id, Resource: role.ID}}
 		gr := sdkGrant.NewGrant(resource, bindingEntitlement, principal,
 			sdkGrant.WithAnnotation(&v2.V1Identifier{
-				Id: fmtGrantIdV1(V1MembershipEntitlementID(resource.Id.Resource), resource.Id.Resource),
+				Id: fmtGrantIdV1(V1MembershipEntitlementID(resource.Id.Resource), role.ID),
 			}),
 		)
 		rv = append(rv, gr)

pkg/connector/resource_sets_bindings.go

@@ -132,7 +132,7 @@ func (rsb *resourceSetsBindingsResourceType) Entitlements(_ context.Context, res
 			sdkEntitlement.WithAnnotation(&v2.V1Identifier{
 				Id: V1MembershipEntitlementID(resource.Id.GetResource()),
 			}),
-			sdkEntitlement.WithGrantableTo(resourceTypeResourceSets),
+			sdkEntitlement.WithGrantableTo(resourceTypeUser, resourceTypeGroup),
 			sdkEntitlement.WithDisplayName(fmt.Sprintf("%s Resource Set Binding Member", resource.DisplayName)),
 			sdkEntitlement.WithDescription(fmt.Sprintf("Member of %s resource-set-binding member in Okta", resource.DisplayName)),
 		),
@@ -226,16 +226,17 @@ func (rsb *resourceSetsBindingsResourceType) unassignMemberFromBinding(ctx conte
 
 func (rsb *resourceSetsBindingsResourceType) Grants(ctx context.Context, resource *v2.Resource, attrs sdkResource.SyncOpAttrs) ([]*v2.Grant, *sdkResource.SyncOpResults, error) {
 	pToken := &attrs.PageToken
-	var (
-		rv        []*v2.Grant
-		principal *v2.Resource
-	)
+	var rv []*v2.Grant
 	bag, _, err := parsePageToken(pToken.Token, resource.Id)
 	if err != nil {
 		return nil, nil, fmt.Errorf("okta-connectorv2: failed to parse page token: %w", err)
 	}
 
 	resourceIDs := strings.Split(resource.Id.Resource, ":")
+	if len(resourceIDs) != resourceMaxLength {
+		return nil, nil, fmt.Errorf("okta-connectorv2: invalid resource set binding id: %s", resource.Id.Resource)
+	}
+
 	resourceSetId := resourceIDs[firstItem]
 	customRoleId := resourceIDs[lastItem]
 	members, _, err := rsb.listMembersOfBinding(ctx, rsb.client, resourceSetId, customRoleId, nil)
@@ -244,6 +245,7 @@ func (rsb *resourceSetsBindingsResourceType) Grants(ctx context.Context, resourc
 	}
 
 	for _, member := range members {
+		var principal *v2.Resource
 		memberHref := strings.Split(member.Links.Self.Href, "/")
 		resourceType := memberHref[len(memberHref)-resourceMaxLength]
 		resourceId := memberHref[len(memberHref)-lastItem]
@@ -260,7 +262,7 @@ func (rsb *resourceSetsBindingsResourceType) Grants(ctx context.Context, resourc
 
 		gr := sdkGrant.NewGrant(resource, entitlementName, principal,
 			sdkGrant.WithAnnotation(&v2.V1Identifier{
-				Id: fmtGrantIdV1(V1MembershipEntitlementID(resource.Id.Resource), resource.Id.Resource),
+				Id: fmtGrantIdV1(V1MembershipEntitlementID(resource.Id.Resource), resourceId),
 			}),
 		)
 		rv = append(rv, gr)
@@ -389,6 +391,10 @@ func (rsb *resourceSetsBindingsResourceType) Get(ctx context.Context, resourceId
 	l.Debug("getting resource set binding", zap.String("resource_set_binding_id", resourceId.Resource))
 
 	resourceIDs := strings.Split(resourceId.Resource, ":")
+	if len(resourceIDs) != resourceMaxLength {
+		return nil, nil, fmt.Errorf("okta-connectorv2: invalid resource set binding id: %s", resourceId.Resource)
+	}
+
 	resourceSetId := resourceIDs[firstItem]
 	customRoleId := resourceIDs[lastItem]
 

pkg/connector/role.go

@@ -428,13 +428,13 @@ func roleGroupGrant(groupID string, resource *v2.Resource, shouldExpand bool) *v
 
 func (g *roleResourceType) Grant(ctx context.Context, principal *v2.Resource, entitlement *v2.Entitlement) (annotations.Annotations, error) {
 	l := ctxzap.Extract(ctx)
-	if principal.Id.ResourceType != resourceTypeUser.Id {
+	if principal.Id.ResourceType != resourceTypeUser.Id && principal.Id.ResourceType != resourceTypeGroup.Id {
 		l.Warn(
 			"okta-connector: only users or groups can be granted role membership",
 			zap.String("principal_type", principal.Id.ResourceType),
 			zap.String("principal_id", principal.Id.Resource),
 		)
-		return nil, fmt.Errorf("okta-connector: only users or groups can be granted repo membership")
+		return nil, fmt.Errorf("okta-connector: only users or groups can be granted role membership")
 	}
 
 	roleId := entitlement.Resource.Id.Resource
@@ -506,6 +506,8 @@ func (g *roleResourceType) Grant(ctx context.Context, principal *v2.Resource, en
 					zap.String("ErrorCode", errOkta.ErrorCode),
 					zap.String("ErrorSummary", errOkta.ErrorSummary),
 				)
+
+				return annotations.New(&v2.GrantAlreadyExists{}), nil
 			}
 
 			return nil, fmt.Errorf("okta-connector: %v", errOkta)
@@ -531,7 +533,7 @@ func (g *roleResourceType) Revoke(ctx context.Context, grant *v2.Grant) (annotat
 	entitlement := grant.Entitlement
 	principal := grant.Principal
 	roleId := ""
-	if principal.Id.ResourceType != resourceTypeUser.Id {
+	if principal.Id.ResourceType != resourceTypeUser.Id && principal.Id.ResourceType != resourceTypeGroup.Id {
 		l.Warn(
 			"okta-connector: only users or groups can have role membership revoked",
 			zap.String("principal_type", principal.Id.ResourceType),

pkg/connector/user.go

@@ -213,14 +213,14 @@ func userName(user *okta.User) (string, string) {
 }
 
 func listUsers(ctx context.Context, client *okta.Client, token *pagination.Token, qp *query.Params) ([]*okta.User, *responseContext, error) {
+	if qp == nil {
+		qp = &query.Params{}
+	}
 	if qp.Search == "" {
 		qp.Search = "status pr" // ListUsers doesn't get deactivated users by default. this should fetch them all
 	}
 
-	uri := usersUrl
-	if qp != nil {
-		uri += qp.String()
-	}
+	uri := usersUrl + qp.String()
 
 	reqUrl, err := url.Parse(uri)
 	if err != nil {
@@ -420,7 +420,7 @@ func getCredentialOption(credentialOptions *v2.LocalCredentialOptions) (*okta.Us
 		return nil, errors.New("unsupported credential options")
 	}
 
-	length := min(8, credentialOptions.GetRandomPassword().GetLength())
+	length := max(8, credentialOptions.GetRandomPassword().GetLength())
 	plaintextPassword, err := crypto.GenerateRandomPassword(&v2.LocalCredentialOptions_RandomPassword{
 		Length: length,
 	})