Skip to content

Commit bc42fbf

Browse files
lostlevelslostlevels
committed
Refactor / remove some role-specific logic out of api layer. Fix panics
when unable to get admin token. Fix bugs found during testing.
1 parent 3bc2377 commit bc42fbf

13 files changed

Lines changed: 360 additions & 202 deletions

‎auth/service/api/v1/auth_service_mock.go‎

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

‎auth/service/api/v1/profile.go‎

Lines changed: 36 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -36,18 +36,14 @@ func (r *Router) ProfileRoutes() []*rest.Route {
3636
}
3737
}
3838

39-
func (r *Router) getProfile(ctx context.Context, userID string) (*user.UserProfile, error) {
40-
// Until seagull migration is complete use UserProfileAccessor() to get a profile instead of the profile within the user itself.
41-
profile, err := r.UserProfileAccessor().FindUserProfile(ctx, userID)
39+
func (r *Router) getProfile(ctx context.Context, userID string) (*user.LegacyUserProfile, error) {
40+
profile, err := r.ProfileAccessor().FindUserProfile(ctx, userID)
4241
if err != nil {
4342
return nil, err
4443
}
4544
if profile == nil {
4645
return nil, user.ErrUserProfileNotFound
4746
}
48-
// Once seagull migration is compelte, we can return
49-
// the profile attached to the user directly via person.Profile
50-
// through r.UserAccessor().FindUserProfile
5147
return profile, nil
5248
}
5349

@@ -56,23 +52,26 @@ func (r *Router) GetProfile(res rest.ResponseWriter, req *rest.Request) {
5652
responder := request.MustNewResponder(res, req)
5753
ctx := req.Context()
5854
userID := req.PathParam("userId")
59-
if r.handledUserNotExists(ctx, responder, userID) {
60-
return
61-
}
62-
6355
profile, err := r.getProfile(ctx, userID)
6456
if err != nil {
65-
r.handleProfileErr(responder, err)
57+
r.handleUserOrProfileErr(responder, err)
6658
return
6759
}
60+
6861
responder.Data(http.StatusOK, profile)
6962
}
7063

7164
func (r *Router) GetUsersWithProfiles(res rest.ResponseWriter, req *rest.Request) {
7265
responder := request.MustNewResponder(res, req)
7366
ctx := req.Context()
7467
targetUserID := req.PathParam("userId")
75-
if r.handledUserNotExists(ctx, responder, targetUserID) {
68+
targetUser, err := r.UserAccessor().FindUserById(ctx, targetUserID)
69+
if err != nil {
70+
r.handleUserOrProfileErr(responder, err)
71+
return
72+
}
73+
if targetUser == nil {
74+
r.handleUserOrProfileErr(responder, user.ErrUserNotFound)
7675
return
7776
}
7877

@@ -127,14 +126,17 @@ func (r *Router) GetUsersWithProfiles(res rest.ResponseWriter, req *rest.Request
127126
if err != nil {
128127
return err
129128
}
130-
profile, err := r.getProfile(ctx, userID)
131-
if stdErrs.Is(err, user.ErrUserProfileNotFound) || profile == nil {
129+
seagullProfile, err := r.getProfile(ctx, userID)
130+
if stdErrs.Is(err, user.ErrUserProfileNotFound) || seagullProfile == nil {
132131
return nil
133132
}
134133
if err != nil {
135134
return err
136135
}
137136
trustorPerms := trustPerms.TrustorPermissions
137+
138+
// TODO: get actual roles
139+
profile := seagullProfile.ToUserProfile(nil)
138140
if trustorPerms == nil || len(*trustorPerms) == 0 {
139141
profile = profile.ClearPatientInfo()
140142
} else {
@@ -156,7 +158,7 @@ func (r *Router) GetUsersWithProfiles(res rest.ResponseWriter, req *rest.Request
156158
})
157159
}
158160
if err := group.Wait(); err != nil {
159-
r.handleProfileErr(responder, err)
161+
r.handleUserOrProfileErr(responder, err)
160162
return
161163
}
162164

@@ -168,73 +170,52 @@ func (r *Router) GetLegacyProfile(res rest.ResponseWriter, req *rest.Request) {
168170
responder := request.MustNewResponder(res, req)
169171
ctx := req.Context()
170172
userID := req.PathParam("userId")
171-
if r.handledUserNotExists(ctx, responder, userID) {
172-
return
173-
}
174-
175173
profile, err := r.getProfile(ctx, userID)
176174
if err != nil {
177-
r.handleProfileErr(responder, err)
175+
r.handleUserOrProfileErr(responder, err)
178176
return
179177
}
180-
responder.Data(http.StatusOK, profile.ToLegacyProfile())
181-
}
182-
183-
func (r *Router) UpdateProfile(res rest.ResponseWriter, req *rest.Request) {
184-
responder := request.MustNewResponder(res, req)
185178

186-
profile := &user.UserProfile{}
187-
if err := request.DecodeRequestBody(req.Request, profile); err != nil {
188-
responder.Error(http.StatusBadRequest, err)
189-
return
190-
}
191-
r.updateProfile(res, req, profile)
179+
responder.Data(http.StatusOK, profile)
192180
}
193181

194182
func (r *Router) UpdateLegacyProfile(res rest.ResponseWriter, req *rest.Request) {
195183
responder := request.MustNewResponder(res, req)
184+
ctx := req.Context()
185+
userID := req.PathParam("userId")
196186

197187
profile := &user.LegacyUserProfile{}
198188
if err := request.DecodeRequestBody(req.Request, profile); err != nil {
199189
responder.Error(http.StatusBadRequest, err)
200190
return
201191
}
202-
r.updateLegacyProfile(res, req, profile)
203-
}
204-
205-
func (r *Router) updateProfile(res rest.ResponseWriter, req *rest.Request, profile *user.UserProfile) {
206-
responder := request.MustNewResponder(res, req)
207-
ctx := req.Context()
208-
userID := req.PathParam("userId")
209192
if err := structValidator.New(log.LoggerFromContext(ctx)).Validate(profile); err != nil {
210193
responder.Error(http.StatusBadRequest, err)
211194
return
212195
}
213-
if r.handledUserNotExists(ctx, responder, userID) {
214-
return
215-
}
216-
// Once seagull migration is complete, we can use r.UserAccessor().UpdateUserProfile.
217-
if err := r.UserProfileAccessor().UpdateUserProfile(ctx, userID, profile); err != nil {
218-
r.handleProfileErr(responder, err)
196+
if err := r.ProfileAccessor().UpdateUserProfile(ctx, userID, profile); err != nil {
197+
r.handleUserOrProfileErr(responder, err)
219198
return
220199
}
221200
responder.Data(http.StatusOK, profile)
222201
}
223202

224-
func (r *Router) updateLegacyProfile(res rest.ResponseWriter, req *rest.Request, profile *user.LegacyUserProfile) {
203+
func (r *Router) UpdateProfile(res rest.ResponseWriter, req *rest.Request) {
225204
responder := request.MustNewResponder(res, req)
226205
ctx := req.Context()
227206
userID := req.PathParam("userId")
228-
if err := structValidator.New(log.LoggerFromContext(ctx)).Validate(profile); err != nil {
207+
208+
profile := &user.UserProfile{}
209+
if err := request.DecodeRequestBody(req.Request, profile); err != nil {
229210
responder.Error(http.StatusBadRequest, err)
230211
return
231212
}
232-
if r.handledUserNotExists(ctx, responder, userID) {
213+
if err := structValidator.New(log.LoggerFromContext(ctx)).Validate(profile); err != nil {
214+
responder.Error(http.StatusBadRequest, err)
233215
return
234216
}
235-
// Once seagull migration is complete, we can use r.UserAccessor().UpdateUserProfile.
236-
if err := r.UserProfileAccessor().UpdateUserProfile(ctx, userID, profile.ToUserProfile()); err != nil {
237-
r.handleProfileErr(responder, err)
217+
if err := r.ProfileAccessor().UpdateUserProfileV2(ctx, userID, profile); err != nil {
218+
r.handleUserOrProfileErr(responder, err)
238219
return
239220
}
240221
responder.Data(http.StatusOK, profile)
@@ -245,17 +226,18 @@ func (r *Router) DeleteProfile(res rest.ResponseWriter, req *rest.Request) {
245226
ctx := req.Context()
246227
userID := req.PathParam("userId")
247228

248-
err := r.UserProfileAccessor().DeleteUserProfile(ctx, userID)
229+
err := r.ProfileAccessor().DeleteUserProfile(ctx, userID)
249230
if err != nil {
250-
r.handleProfileErr(responder, err)
231+
r.handleUserOrProfileErr(responder, err)
251232
return
252233
}
253234
responder.Empty(http.StatusOK)
254235
}
255236

256-
func (r *Router) handleProfileErr(responder *request.Responder, err error) {
237+
func (r *Router) handleUserOrProfileErr(responder *request.Responder, err error) {
257238
switch {
258239
case stdErrs.Is(err, user.ErrUserNotFound), stdErrs.Is(err, user.ErrUserProfileNotFound):
240+
// Many of the seagull clients don't treat 404 as an error so return 404 as is
259241
responder.Empty(http.StatusNotFound)
260242
return
261243
default:
@@ -266,7 +248,7 @@ func (r *Router) handleProfileErr(responder *request.Responder, err error) {
266248
func (r *Router) handledUserNotExists(ctx context.Context, responder *request.Responder, userID string) (handled bool) {
267249
person, err := r.UserAccessor().FindUserById(ctx, userID)
268250
if err != nil {
269-
r.handleProfileErr(responder, err)
251+
r.handleUserOrProfileErr(responder, err)
270252
return true
271253
}
272254
if person == nil {

‎auth/service/service.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ type Service interface {
2323
Domain() string
2424
AuthStore() store.Store
2525
UserAccessor() user.UserAccessor
26-
UserProfileAccessor() user.UserProfileAccessor // UserProfileAccessor is separate from UserAccessor while the seagull migration is in progress because the user returned from UserAccessor is the keycloak user and their profile may not have been migrated yet
26+
ProfileAccessor() user.ProfileAccessor
2727
PermissionsClient() permission.ExtendedClient
2828

2929
ProviderFactory() provider.Factory

‎auth/service/service/service.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ type Service struct {
7070
appValidator *appvalidate.Validator
7171
partnerSecrets *appvalidate.PartnerSecrets
7272
userAccessor user.UserAccessor
73-
userProfileAccessor user.UserProfileAccessor
73+
userProfileAccessor user.ProfileAccessor
7474
permsClient *permissionClient.Client
7575
twiistServiceAccountAuthorizer twiist.ServiceAccountAuthorizer
7676
}
@@ -197,7 +197,7 @@ func (s *Service) UserAccessor() user.UserAccessor {
197197
return s.userAccessor
198198
}
199199

200-
func (s *Service) UserProfileAccessor() user.UserProfileAccessor {
200+
func (s *Service) ProfileAccessor() user.ProfileAccessor {
201201
return s.userProfileAccessor
202202
}
203203

@@ -567,7 +567,7 @@ func (s *Service) initializeUserProfileAccessor(userAccessor user.UserAccessor)
567567
return errors.Wrap(err, "unable to create fallback user profile repository")
568568
}
569569

570-
s.userProfileAccessor = user.NewFallbackLegacyUserAccessor(repo, userAccessor)
570+
s.userProfileAccessor = user.NewFallbackLegacyUserAccessor(repo, userAccessor, userAccessor)
571571
return nil
572572
}
573573

‎auth/store/mongo/legacy_seagull_profile_repository.go‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -60,15 +60,14 @@ func (p *LegacySeagullProfileRepository) FindUserProfile(ctx context.Context, us
6060
return doc.ToLegacyProfile()
6161
}
6262

63-
func (p *LegacySeagullProfileRepository) UpdateUserProfile(ctx context.Context, userID string, profile *user.UserProfile) error {
63+
func (p *LegacySeagullProfileRepository) UpdateUserProfile(ctx context.Context, userID string, profile *user.LegacyUserProfile) error {
6464
if ctx == nil {
6565
return errors.New("context is missing")
6666
}
6767
if userID == "" {
6868
return errors.New("user id is missing")
6969
}
70-
legacyProfile := profile.ToLegacyProfile()
71-
if err := structureValidator.New(log.LoggerFromContext(ctx)).Validate(legacyProfile); err != nil {
70+
if err := structureValidator.New(log.LoggerFromContext(ctx)).Validate(profile); err != nil {
7271
return err
7372
}
7473
var doc user.LegacySeagullDocument
@@ -87,7 +86,7 @@ func (p *LegacySeagullProfileRepository) UpdateUserProfile(ctx context.Context,
8786
}
8887

8988
// This will create a new value even if doc.Value is empty
90-
updatedValueRaw, err := user.AddProfileToSeagullValue(doc.Value, legacyProfile)
89+
updatedValueRaw, err := user.AddProfileToSeagullValue(doc.Value, profile)
9190
if err != nil {
9291
return err
9392
}

0 commit comments

Comments
 (0)