Skip to content

Commit 2db71b3

Browse files
EmyrkEmyrk
committed
chore: implement organization sync and create idpsync package
IDP sync code should be refactored to be contained in it's own package. This will make it easier to maintain and test moving forward.
1 parent 6fe4cc0 commit 2db71b3

File tree

7 files changed

+279
-57
lines changed

7 files changed

+279
-57
lines changed

‎coderd/database/dbmetrics/dbmetrics.go

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/models.go

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/querier.go

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/idpsync/idpsync.go

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
package idpsync
2+
3+
import (
4+
"net/http"
5+
"strings"
6+
7+
"github.com/google/uuid"
8+
"golang.org/x/xerrors"
9+
10+
"cdr.dev/slog"
11+
"github.com/coder/coder/v2/coderd/entitlements"
12+
"github.com/coder/coder/v2/coderd/httpapi"
13+
"github.com/coder/coder/v2/codersdk"
14+
"github.com/coder/coder/v2/site"
15+
)
16+
17+
// IDPSync is the configuration for syncing user information from an external
18+
// IDP. All related code to syncing user information should be in this package.
19+
type IDPSync struct {
20+
logger slog.Logger
21+
entitlements *entitlements.Set
22+
23+
// OrganizationField selects the claim field to be used as the created user's
24+
// organizations. If the field is the empty string, then no organization updates
25+
// will ever come from the OIDC provider.
26+
OrganizationField string
27+
// OrganizationMapping controls how organizations returned by the OIDC provider get mapped
28+
OrganizationMapping map[string][]uuid.UUID
29+
// OrganizationAlwaysAssign will ensure all users that authenticate will be
30+
// placed into the specified organizations.
31+
OrganizationAlwaysAssign []uuid.UUID
32+
}
33+
34+
func NewSync(logger slog.Logger, set *entitlements.Set) *IDPSync {
35+
return &IDPSync{
36+
logger: logger.Named("idp-sync"),
37+
entitlements: set,
38+
}
39+
}
40+
41+
// ParseStringSliceClaim parses the claim for groups and roles, expected []string.
42+
//
43+
// Some providers like ADFS return a single string instead of an array if there
44+
// is only 1 element. So this function handles the edge cases.
45+
func ParseStringSliceClaim(claim interface{}) ([]string, error) {
46+
groups := make([]string, 0)
47+
if claim == nil {
48+
return groups, nil
49+
}
50+
51+
// The simple case is the type is exactly what we expected
52+
asStringArray, ok := claim.([]string)
53+
if ok {
54+
return asStringArray, nil
55+
}
56+
57+
asArray, ok := claim.([]interface{})
58+
if ok {
59+
for i, item := range asArray {
60+
asString, ok := item.(string)
61+
if !ok {
62+
return nil, xerrors.Errorf("invalid claim type. Element %d expected a string, got: %T", i, item)
63+
}
64+
groups = append(groups, asString)
65+
}
66+
return groups, nil
67+
}
68+
69+
asString, ok := claim.(string)
70+
if ok {
71+
if asString == "" {
72+
// Empty string should be 0 groups.
73+
return []string{}, nil
74+
}
75+
// If it is a single string, first check if it is a csv.
76+
// If a user hits this, it is likely a misconfiguration and they need
77+
// to reconfigure their IDP to send an array instead.
78+
if strings.Contains(asString, ",") {
79+
return nil, xerrors.Errorf("invalid claim type. Got a csv string (%q), change this claim to return an array of strings instead.", asString)
80+
}
81+
return []string{asString}, nil
82+
}
83+
84+
// Not sure what the user gave us.
85+
return nil, xerrors.Errorf("invalid claim type. Expected an array of strings, got: %T", claim)
86+
}
87+
88+
// HttpError is a helper struct for returning errors from the IDP sync process.
89+
// A regular error is not sufficient because many of these errors are surfaced
90+
// to a user logging in, and the errors should be descriptive.
91+
type HttpError struct {
92+
Code int
93+
Msg string
94+
Detail string
95+
RenderStaticPage bool
96+
RenderDetailMarkdown bool
97+
}
98+
99+
func (e HttpError) Write(rw http.ResponseWriter, r *http.Request) {
100+
if e.RenderStaticPage {
101+
site.RenderStaticErrorPage(rw, r, site.ErrorPageData{
102+
Status: e.Code,
103+
HideStatus: true,
104+
Title: e.Msg,
105+
Description: e.Detail,
106+
RetryEnabled: false,
107+
DashboardURL: "/login",
108+
109+
RenderDescriptionMarkdown: e.RenderDetailMarkdown,
110+
})
111+
return
112+
}
113+
httpapi.Write(r.Context(), rw, e.Code, codersdk.Response{
114+
Message: e.Msg,
115+
Detail: e.Detail,
116+
})
117+
}
118+
119+
func (e HttpError) Error() string {
120+
if e.Detail != "" {
121+
return e.Detail
122+
}
123+
124+
return e.Msg
125+
}

‎coderd/userauth_internal_test.go renamed to ‎coderd/idpsync/idpsync_test.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
1-
package coderd
1+
package idpsync_test
22

33
import (
44
"encoding/json"
55
"testing"
66

77
"github.com/stretchr/testify/require"
8+
9+
"github.com/coder/coder/v2/coderd/idpsync"
810
)
911

1012
func TestParseStringSliceClaim(t *testing.T) {
@@ -123,7 +125,7 @@ func TestParseStringSliceClaim(t *testing.T) {
123125
require.NoError(t, err, "unmarshal json claim")
124126
}
125127

126-
found, err := parseStringSliceClaim(c.GoClaim)
128+
found, err := idpsync.ParseStringSliceClaim(c.GoClaim)
127129
if c.ErrorExpected {
128130
require.Error(t, err)
129131
} else {

‎coderd/idpsync/organization.go

Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,127 @@
1+
package idpsync
2+
3+
import (
4+
"context"
5+
"database/sql"
6+
"net/http"
7+
8+
"github.com/google/uuid"
9+
"golang.org/x/xerrors"
10+
11+
"cdr.dev/slog"
12+
"github.com/coder/coder/v2/coderd/database"
13+
"github.com/coder/coder/v2/coderd/database/db2sdk"
14+
"github.com/coder/coder/v2/coderd/database/dbauthz"
15+
"github.com/coder/coder/v2/coderd/database/dbtime"
16+
"github.com/coder/coder/v2/coderd/util/slice"
17+
)
18+
19+
func (s IDPSync) ParseOrganizationClaims(ctx context.Context, mergedClaims map[string]interface{}) (OrganizationParams, *HttpError) {
20+
// nolint:gocritic // all syncing is done as a system user
21+
ctx = dbauthz.AsSystemRestricted(ctx)
22+
23+
// Copy in the always included static set of organizations.
24+
userOrganizations := make([]uuid.UUID, len(s.OrganizationAlwaysAssign))
25+
copy(userOrganizations, s.OrganizationAlwaysAssign)
26+
27+
// Pull extra organizations from the claims.
28+
if s.OrganizationField != "" {
29+
organizationRaw, ok := mergedClaims[s.OrganizationField]
30+
if ok {
31+
parsedOrganizations, err := ParseStringSliceClaim(organizationRaw)
32+
if err != nil {
33+
return OrganizationParams{}, &HttpError{
34+
Code: http.StatusBadRequest,
35+
Msg: "Failed to sync organizations from the OIDC claims",
36+
Detail: err.Error(),
37+
RenderStaticPage: false,
38+
RenderDetailMarkdown: false,
39+
}
40+
}
41+
42+
// Keep track of which claims are not mapped for debugging purposes.
43+
var ignored []string
44+
for _, parsedOrg := range parsedOrganizations {
45+
if mappedOrganization, ok := s.OrganizationMapping[parsedOrg]; ok {
46+
// parsedOrg is in the mapping, so add the mapped organizations to the
47+
// user's organizations.
48+
userOrganizations = append(userOrganizations, mappedOrganization...)
49+
} else {
50+
ignored = append(ignored, parsedOrg)
51+
}
52+
}
53+
54+
s.logger.Debug(ctx, "parsed organizations from claim",
55+
slog.F("len", len(parsedOrganizations)),
56+
slog.F("ignored", ignored),
57+
slog.F("organizations", parsedOrganizations),
58+
)
59+
}
60+
}
61+
62+
return OrganizationParams{
63+
Organizations: userOrganizations,
64+
}, nil
65+
}
66+
67+
type OrganizationParams struct {
68+
// Organizations is the list of organizations the user should be a member of
69+
// assuming syncing is turned on.
70+
Organizations []uuid.UUID
71+
}
72+
73+
func (s IDPSync) SyncOrganizations(ctx context.Context, tx database.Store, user database.User, params OrganizationParams) error {
74+
// nolint:gocritic // all syncing is done as a system user
75+
ctx = dbauthz.AsSystemRestricted(ctx)
76+
77+
existingOrgs, err := tx.GetOrganizationsByUserID(ctx, user.ID)
78+
if err != nil {
79+
return xerrors.Errorf("failed to get user organizations: %w", err)
80+
}
81+
82+
existingOrgIDs := db2sdk.List(existingOrgs, func(org database.Organization) uuid.UUID {
83+
return org.ID
84+
})
85+
86+
// Find the difference in the expected and the existing orgs, and
87+
// correct the set of orgs the user is a member of.
88+
add, remove := slice.SymmetricDifference(existingOrgIDs, params.Organizations)
89+
notExists := make([]uuid.UUID, 0)
90+
for _, orgID := range add {
91+
//nolint:gocritic // System actor being used to assign orgs
92+
_, err := tx.InsertOrganizationMember(dbauthz.AsSystemRestricted(ctx), database.InsertOrganizationMemberParams{
93+
OrganizationID: orgID,
94+
UserID: user.ID,
95+
CreatedAt: dbtime.Now(),
96+
UpdatedAt: dbtime.Now(),
97+
Roles: []string{},
98+
})
99+
if err != nil {
100+
if xerrors.Is(err, sql.ErrNoRows) {
101+
notExists = append(notExists, orgID)
102+
continue
103+
}
104+
return xerrors.Errorf("add user to organization: %w", err)
105+
}
106+
}
107+
108+
for _, orgID := range remove {
109+
//nolint:gocritic // System actor being used to assign orgs
110+
err := tx.DeleteOrganizationMember(dbauthz.AsSystemRestricted(ctx), database.DeleteOrganizationMemberParams{
111+
OrganizationID: orgID,
112+
UserID: user.ID,
113+
})
114+
if err != nil {
115+
return xerrors.Errorf("remove user from organization: %w", err)
116+
}
117+
}
118+
119+
if len(notExists) > 0 {
120+
s.logger.Debug(ctx, "organizations do not exist but attempted to use in org sync",
121+
slog.F("not_found", notExists),
122+
slog.F("user_id", user.ID),
123+
slog.F("username", user.Username),
124+
)
125+
}
126+
return nil
127+
}

0 commit comments

Comments
 (0)