Skip to content

Commit e08aaf8

Browse files
kylecarbskylecarbs
committed
feat: Add profile pictures to OAuth users
This supports GitHub and OIDC login for profile pictures!
1 parent aae5747 commit e08aaf8

File tree

23 files changed

+139
-33
lines changed

23 files changed

+139
-33
lines changed

‎coderd/database/databasefake/databasefake.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1859,6 +1859,7 @@ func (q *fakeQuerier) UpdateUserProfile(_ context.Context, arg database.UpdateUs
18591859
}
18601860
user.Email = arg.Email
18611861
user.Username = arg.Username
1862+
user.AvatarURL = arg.AvatarURL
18621863
q.users[index] = user
18631864
return user, nil
18641865
}

‎coderd/database/dump.sql

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

‎coderd/database/migrations/000044_user_avatars.down.sql

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
ALTER TABLE users
2+
DROP COLUMN avatar_url;

‎coderd/database/migrations/000044_user_avatars.up.sql

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
ALTER TABLE users
2+
ADD COLUMN avatar_url varchar(64);

‎coderd/database/models.go

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

‎coderd/database/queries.sql.go

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

‎coderd/database/queries/users.sql

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,8 @@ UPDATE
5757
SET
5858
email = $2,
5959
username = $3,
60-
updated_at = $4
60+
avatar_url = $4,
61+
updated_at = $5
6162
WHERE
6263
id = $1 RETURNING *;
6364

‎coderd/database/sqlc.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ packages:
1818

1919
rename:
2020
api_key: APIKey
21+
avatar_url: AvatarURL
2122
login_type_oidc: LoginTypeOIDC
2223
oauth_access_token: OAuthAccessToken
2324
oauth_expiry: OAuthExpiry

‎coderd/userauth.go

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,7 @@ func (api *API) userOAuth2Github(rw http.ResponseWriter, r *http.Request) {
144144
AllowSignups: api.GithubOAuth2Config.AllowSignups,
145145
Email: verifiedEmail.GetEmail(),
146146
Username: ghUser.GetLogin(),
147+
AvatarURL: ghUser.GetAvatarURL(),
147148
})
148149
var httpErr httpError
149150
if xerrors.As(err, &httpErr) {
@@ -207,6 +208,7 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) {
207208
Email string `json:"email"`
208209
Verified bool `json:"email_verified"`
209210
Username string `json:"preferred_username"`
211+
Picture string `json:"picture"`
210212
}
211213
err = idToken.Claims(&claims)
212214
if err != nil {
@@ -256,6 +258,7 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) {
256258
AllowSignups: api.OIDCConfig.AllowSignups,
257259
Email: claims.Email,
258260
Username: claims.Username,
261+
AvatarURL: claims.Picture,
259262
})
260263
var httpErr httpError
261264
if xerrors.As(err, &httpErr) {
@@ -292,6 +295,7 @@ type oauthLoginParams struct {
292295
AllowSignups bool
293296
Email string
294297
Username string
298+
AvatarURL string
295299
}
296300

297301
type httpError struct {
@@ -410,13 +414,27 @@ func (api *API) oauthLogin(r *http.Request, params oauthLoginParams) (*http.Cook
410414
}
411415
}
412416

417+
needsUpdate := false
418+
if user.AvatarURL.String != params.AvatarURL {
419+
user.AvatarURL = sql.NullString{
420+
String: params.AvatarURL,
421+
Valid: true,
422+
}
423+
needsUpdate = true
424+
}
425+
413426
// If the upstream email or username has changed we should mirror
414427
// that in Coder. Many enterprises use a user's email/username as
415428
// security auditing fields so they need to stay synced.
416429
// NOTE: username updating has been halted since it can have infrastructure
417430
// provisioning consequences (updates to usernames may delete persistent
418431
// resources such as user home volumes).
419432
if user.Email != params.Email {
433+
user.Email = params.Email
434+
needsUpdate = true
435+
}
436+
437+
if needsUpdate {
420438
// TODO(JonA): Since we're processing updates to a user's upstream
421439
// email/username, it's possible for a different built-in user to
422440
// have already claimed the username.
@@ -425,9 +443,10 @@ func (api *API) oauthLogin(r *http.Request, params oauthLoginParams) (*http.Cook
425443
// user and changes their username.
426444
user, err = tx.UpdateUserProfile(ctx, database.UpdateUserProfileParams{
427445
ID: user.ID,
428-
Email: params.Email,
446+
Email: user.Email,
429447
Username: user.Username,
430448
UpdatedAt: database.Now(),
449+
AvatarURL: user.AvatarURL,
431450
})
432451
if err != nil {
433452
return xerrors.Errorf("update user profile: %w", err)

‎coderd/userauth_test.go

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -226,10 +226,11 @@ func TestUserOAuth2Github(t *testing.T) {
226226
},
227227
}}, nil
228228
},
229-
AuthenticatedUser: func(ctx context.Context, client *http.Client) (*github.User, error) {
229+
AuthenticatedUser: func(ctx context.Context, _ *http.Client) (*github.User, error) {
230230
return &github.User{
231-
Login: github.String("kyle"),
232-
ID: i64ptr(1234),
231+
Login: github.String("kyle"),
232+
ID: i64ptr(1234),
233+
AvatarURL: github.String("/hello-world"),
233234
}, nil
234235
},
235236
ListEmails: func(ctx context.Context, client *http.Client) ([]*github.UserEmail, error) {
@@ -249,6 +250,7 @@ func TestUserOAuth2Github(t *testing.T) {
249250
require.NoError(t, err)
250251
require.Equal(t, "kyle@coder.com", user.Email)
251252
require.Equal(t, "kyle", user.Username)
253+
require.Equal(t, "/hello-world", user.AvatarURL)
252254
})
253255
t.Run("SignupAllowedTeam", func(t *testing.T) {
254256
t.Parallel()
@@ -297,6 +299,7 @@ func TestUserOIDC(t *testing.T) {
297299
AllowSignups bool
298300
EmailDomain string
299301
Username string
302+
AvatarURL string
300303
StatusCode int
301304
}{{
302305
Name: "EmailNotVerified",
@@ -357,6 +360,18 @@ func TestUserOIDC(t *testing.T) {
357360
Username: "kyle",
358361
AllowSignups: true,
359362
StatusCode: http.StatusTemporaryRedirect,
363+
}, {
364+
Name: "WithPicture",
365+
Claims: jwt.MapClaims{
366+
"email": "kyle@kwc.io",
367+
"email_verified": true,
368+
"username": "kyle",
369+
"picture": "/example.png",
370+
},
371+
Username: "kyle",
372+
AllowSignups: true,
373+
AvatarURL: "/example.png",
374+
StatusCode: http.StatusTemporaryRedirect,
360375
}} {
361376
tc := tc
362377
t.Run(tc.Name, func(t *testing.T) {
@@ -379,6 +394,13 @@ func TestUserOIDC(t *testing.T) {
379394
require.NoError(t, err)
380395
require.Equal(t, tc.Username, user.Username)
381396
}
397+
398+
if tc.AvatarURL != "" {
399+
client.SessionToken = resp.Cookies()[0].Value
400+
user, err := client.User(ctx, "me")
401+
require.NoError(t, err)
402+
require.Equal(t, tc.AvatarURL, user.AvatarURL)
403+
}
382404
})
383405
}
384406

‎coderd/users.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -391,6 +391,7 @@ func (api *API) putUserProfile(rw http.ResponseWriter, r *http.Request) {
391391
updatedUserProfile, err := api.Database.UpdateUserProfile(r.Context(), database.UpdateUserProfileParams{
392392
ID: user.ID,
393393
Email: user.Email,
394+
AvatarURL: user.AvatarURL,
394395
Username: params.Username,
395396
UpdatedAt: database.Now(),
396397
})
@@ -1075,6 +1076,7 @@ func convertUser(user database.User, organizationIDs []uuid.UUID) codersdk.User
10751076
Status: codersdk.UserStatus(user.Status),
10761077
OrganizationIDs: organizationIDs,
10771078
Roles: make([]codersdk.Role, 0, len(user.RBACRoles)),
1079+
AvatarURL: user.AvatarURL.String,
10781080
}
10791081

10801082
for _, roleName := range user.RBACRoles {

‎codersdk/users.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ type User struct {
5050
Status UserStatus `json:"status" table:"status"`
5151
OrganizationIDs []uuid.UUID `json:"organization_ids"`
5252
Roles []Role `json:"roles"`
53+
AvatarURL string `json:"avatar_url"`
5354
}
5455

5556
type APIKey struct {

‎enterprise/audit/table.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@ var AuditableResources = auditMap(map[any]map[string]Action{
8383
"status": ActionTrack,
8484
"rbac_roles": ActionTrack,
8585
"login_type": ActionIgnore,
86+
"avatar_url": ActionIgnore,
8687
},
8788
&database.Workspace{}: {
8889
"id": ActionTrack,

0 commit comments

Comments
 (0)