appview: don't trust `oauth.ClientSessionData` #1188

+83 -55

9 changed files

Diff round #2

expand all

appview

ingester.go

oauth

accounts.go

accounts_test.go

oauth.go

pages

funcmap.go

templates

user

settings

profile.html

settings

danger.go

settings.go

state

router.go

+6 -1

appview/ingester.go

··· 50 50 l := i.Logger.With("kind", e.Kind) 51 51 switch e.Kind { 52 52 case jmodels.EventKindAccount: 53 - if !e.Account.Active && *e.Account.Status == "deactivated" { 53 + // TODO: sync account state to db 54 + if e.Account.Active { 55 + break 56 + } 57 + // TODO: revoke sessions by DID 58 + if *e.Account.Status == "deactivated" { 54 59 err = i.IdResolver.InvalidateIdent(ctx, e.Account.Did) 55 60 } 56 61 case jmodels.EventKindIdentity:

-7

appview/oauth/accounts.go

··· 34 34 return m.Active.Did 35 35 } 36 36 37 - func (m *MultiAccountUser) Pds() string { 38 - if m.Active == nil { 39 - return "" 40 - } 41 - return m.Active.Pds 42 - } 43 - 44 37 func (o *OAuth) GetAccounts(r *http.Request) *AccountRegistry { 45 38 session, err := o.SessStore.Get(r, AccountsName) 46 39 if err != nil || session.IsNew {

+1 -1

appview/oauth/accounts_test.go

··· 249 249 func TestMultiAccountUser_Did(t *testing.T) { 250 250 t.Run("with active user", func(t *testing.T) { 251 251 user := &MultiAccountUser{ 252 - Active: &User{Did: "did:plc:test", Pds: "https://bsky.social"}, 252 + Active: &User{Did: "did:plc:test"}, 253 253 } 254 254 if user.Did() != "did:plc:test" { 255 255 t.Errorf("Did() = %s, want did:plc:test", user.Did())

-2

appview/oauth/oauth.go

··· 239 239 240 240 type User struct { 241 241 Did string 242 - Pds string 243 242 } 244 243 245 244 func (o *OAuth) GetUser(r *http.Request) *User { ··· 250 249 251 250 return &User{ 252 251 Did: sess.Data.AccountDID.String(), 253 - Pds: sess.Data.HostURL, 254 252 } 255 253 } 256 254

appview/pages/funcmap.go

··· 77 77 78 78 return identity.Handle.String() 79 79 }, 80 + "resolvePds": func(s string) string { 81 + identity, err := p.resolver.ResolveIdent(context.Background(), s) 82 + if err != nil { 83 + return "" 84 + } 85 + return identity.PDSEndpoint() 86 + }, 80 87 "ownerSlashRepo": func(repo *models.Repo) string { 81 88 ownerId, err := p.resolver.ResolveIdent(context.Background(), repo.Did) 82 89 if err != nil {

+5 -4

appview/pages/templates/user/settings/profile.html

··· 49 49 </div> 50 50 <div class="flex flex-col gap-1 p-4"> 51 51 <span class="text-sm text-gray-500 dark:text-gray-400">Personal Data Server (PDS)</span> 52 - <span class="font-bold">{{ .LoggedInUser.Pds }}</span> 52 + <span class="font-bold">{{ resolvePds .LoggedInUser.Did }}</span> 53 53 </div> 54 54 </div> 55 55 {{ if and .IsTnglSh .HandleOpen }} ··· 132 132 {{ end }} 133 133 134 134 {{ define "accountActions" }} 135 + {{ $isDeactivated := (and .IsTnglSh .IsDeactivated) }} 135 136 <div> 136 137 <h2 class="text-sm uppercase font-bold">Account</h2> 137 - {{ if .IsDeactivated }} 138 + {{ if $isDeactivated }} 138 139 <div class="mt-2 p-3 bg-amber-50 dark:bg-amber-900/20 border border-amber-200 dark:border-amber-700 rounded text-sm text-amber-700 dark:text-amber-300"> 139 140 Your account is deactivated. Your profile and repositories are currently inaccessible. Reactivate to restore access. 140 141 </div> ··· 147 148 {{ i "key" "size-4" }} 148 149 change password 149 150 </button> 150 - {{ if .IsDeactivated }} 151 + {{ if $isDeactivated }} 151 152 <button 152 153 popovertarget="reactivate-modal" 153 154 popovertargetaction="toggle" ··· 199 200 </div> 200 201 </div> 201 202 202 - {{ if .IsDeactivated }} 203 + {{ if $isDeactivated }} 203 204 <div 204 205 id="reactivate-modal" 205 206 popover

+15 -9

appview/settings/danger.go

··· 8 8 "time" 9 9 10 10 comatproto "github.com/bluesky-social/indigo/api/atproto" 11 + "github.com/bluesky-social/indigo/atproto/syntax" 11 12 "github.com/bluesky-social/indigo/xrpc" 12 13 ) 13 14 ··· 58 59 59 60 func (s *Settings) requestPasswordReset(w http.ResponseWriter, r *http.Request) { 60 61 user := s.OAuth.GetMultiAccountUser(r) 61 - if !s.Config.Pds.IsTnglShUser(user.Pds()) { 62 + if isTngl, _ := s.isTnglShUser(r.Context(), syntax.DID(user.Active.Did)); !isTngl { 62 63 s.Pages.Notice(w, "password-error", "Only available for tngl.sh accounts.") 63 64 return 64 65 } ··· 99 100 100 101 func (s *Settings) resetPassword(w http.ResponseWriter, r *http.Request) { 101 102 user := s.OAuth.GetMultiAccountUser(r) 102 - if !s.Config.Pds.IsTnglShUser(user.Pds()) { 103 + if isTngl, _ := s.isTnglShUser(r.Context(), syntax.DID(user.Active.Did)); !isTngl { 103 104 s.Pages.Notice(w, "password-error", "Only available for tngl.sh accounts.") 104 105 return 105 106 } ··· 133 134 134 135 func (s *Settings) deactivateAccount(w http.ResponseWriter, r *http.Request) { 135 136 user := s.OAuth.GetMultiAccountUser(r) 136 - if !s.Config.Pds.IsTnglShUser(user.Pds()) { 137 + if isTngl, _ := s.isTnglShUser(r.Context(), syntax.DID(user.Active.Did)); !isTngl { 137 138 s.Pages.Notice(w, "deactivate-error", "Only available for tngl.sh accounts.") 138 139 return 139 140 } ··· 171 172 172 173 func (s *Settings) requestAccountDelete(w http.ResponseWriter, r *http.Request) { 173 174 user := s.OAuth.GetMultiAccountUser(r) 174 - if !s.Config.Pds.IsTnglShUser(user.Pds()) { 175 + if isTngl, _ := s.isTnglShUser(r.Context(), syntax.DID(user.Active.Did)); !isTngl { 175 176 s.Pages.Notice(w, "delete-error", "Only available for tngl.sh accounts.") 176 177 return 177 178 } ··· 203 204 204 205 func (s *Settings) deleteAccount(w http.ResponseWriter, r *http.Request) { 205 206 user := s.OAuth.GetMultiAccountUser(r) 206 - if !s.Config.Pds.IsTnglShUser(user.Pds()) { 207 + if isTngl, _ := s.isTnglShUser(r.Context(), syntax.DID(user.Active.Did)); !isTngl { 207 208 s.Pages.Notice(w, "delete-error", "Only available for tngl.sh accounts.") 208 209 return 209 210 } ··· 243 244 s.Pages.HxRedirect(w, "/") 244 245 } 245 246 246 - func (s *Settings) isAccountDeactivated(ctx context.Context, did, pdsHost string) bool { 247 + func (s *Settings) isAccountDeactivated(ctx context.Context, did syntax.DID) bool { 248 + ident, err := s.IdResolver.ResolveIdent(ctx, did.String()) 249 + if err != nil { 250 + s.Logger.Error("failed to resolve user did", "err", err) 251 + return false 252 + } 247 253 client := &xrpc.Client{ 248 - Host: pdsHost, 254 + Host: ident.PDSEndpoint(), 249 255 Client: &http.Client{Timeout: 5 * time.Second}, 250 256 } 251 257 252 - _, err := comatproto.RepoDescribeRepo(ctx, client, did) 258 + _, err = comatproto.RepoDescribeRepo(ctx, client, did.String()) 253 259 if err == nil { 254 260 return false 255 261 } ··· 263 269 264 270 func (s *Settings) reactivateAccount(w http.ResponseWriter, r *http.Request) { 265 271 user := s.OAuth.GetMultiAccountUser(r) 266 - if !s.Config.Pds.IsTnglShUser(user.Pds()) { 272 + if isTngl, _ := s.isTnglShUser(r.Context(), syntax.DID(user.Active.Did)); !isTngl { 267 273 s.Pages.Notice(w, "reactivate-error", "Only available for tngl.sh accounts.") 268 274 return 269 275 }

+42 -25

appview/settings/settings.go

··· 25 25 "tangled.org/core/appview/oauth" 26 26 "tangled.org/core/appview/pages" 27 27 "tangled.org/core/appview/sites" 28 + "tangled.org/core/idresolver" 28 29 "tangled.org/core/tid" 29 30 30 31 comatproto "github.com/bluesky-social/indigo/api/atproto" ··· 36 37 ) 37 38 38 39 type Settings struct { 39 - Db *db.DB 40 - OAuth *oauth.OAuth 41 - Pages *pages.Pages 42 - Config *config.Config 43 - CfClient *cloudflare.Client 44 - Logger *slog.Logger 40 + Db *db.DB 41 + OAuth *oauth.OAuth 42 + Pages *pages.Pages 43 + Config *config.Config 44 + CfClient *cloudflare.Client 45 + IdResolver *idresolver.Resolver 46 + Logger *slog.Logger 45 47 } 46 48 47 49 func (s *Settings) Router() http.Handler { ··· 92 94 return r 93 95 } 94 96 97 + func (s *Settings) isTnglHandle(ctx context.Context, did syntax.DID) (bool, error) { 98 + userIdent, err := s.IdResolver.ResolveIdent(ctx, did.String()) 99 + if err != nil { 100 + s.Logger.Error("failed to resolve user ident", "err", err) 101 + return false, err 102 + } 103 + return strings.HasSuffix(userIdent.Handle.String(), s.Config.Pds.UserDomain), nil 104 + } 105 + 106 + func (s *Settings) isTnglShUser(ctx context.Context, did syntax.DID) (bool, error) { 107 + userIdent, err := s.IdResolver.ResolveIdent(ctx, did.String()) 108 + if err != nil { 109 + s.Logger.Error("failed to resolve user ident", "err", err) 110 + return false, err 111 + } 112 + return strings.TrimRight(userIdent.PDSEndpoint(), "/") == strings.TrimRight(s.Config.Pds.Host, "/"), nil 113 + } 114 + 95 115 func (s *Settings) sitesSettings(w http.ResponseWriter, r *http.Request) { 96 116 user := s.OAuth.GetMultiAccountUser(r) 97 117 ··· 103 123 104 124 // determine whether the active account has a tngl.sh handle, in which 105 125 // case their sites domain is automatically their handle domain. 106 - isTnglHandle := false 107 - for _, acc := range user.Accounts { 108 - if acc.Did == user.Active.Did { 109 - isTnglHandle = strings.HasSuffix(acc.Handle, s.Config.Pds.UserDomain) 110 - break 111 - } 112 - } 126 + isTnglHandle, _ := s.isTnglHandle(r.Context(), syntax.DID(user.Active.Did)) 113 127 114 128 s.Pages.UserSiteSettings(w, pages.UserSiteSettingsParams{ 115 129 LoggedInUser: user, ··· 179 193 return 180 194 } 181 195 182 - for _, acc := range user.Accounts { 183 - if acc.Did == user.Active.Did { 184 - if strings.HasSuffix(acc.Handle, s.Config.Pds.UserDomain) { 185 - s.Pages.Notice(w, "settings-sites-error", "Your tngl.sh domain is tied to your handle and cannot be released here.") 186 - return 187 - } 188 - break 189 - } 196 + isTnglHandle, err := s.isTnglHandle(r.Context(), syntax.DID(user.Active.Did)) 197 + if err != nil { 198 + s.Pages.Notice(w, "settings-sites-error", "Unable to resolve user identity") 199 + return 200 + } 201 + if isTnglHandle { 202 + s.Pages.Notice(w, "settings-sites-error", "Your tngl.sh domain is tied to your handle and cannot be released here.") 203 + return 190 204 } 191 205 192 206 if err := db.ReleaseDomain(s.Db, user.Active.Did, domain); err != nil { ··· 251 265 log.Printf("failed to get users punchcard preferences: %s", err) 252 266 } 253 267 254 - isDeactivated := s.Config.Pds.IsTnglShUser(user.Pds()) && s.isAccountDeactivated(r.Context(), user.Did(), user.Pds()) 268 + isTnglSh, err := s.isTnglShUser(r.Context(), syntax.DID(user.Active.Did)) 269 + 270 + // TODO: bring the user state from DB instead of PDS request 271 + isDeactivated := s.isAccountDeactivated(r.Context(), syntax.DID(user.Active.Did)) 255 272 256 273 s.Pages.UserProfileSettings(w, pages.UserProfileSettingsParams{ 257 274 LoggedInUser: user, 258 275 PunchcardPreference: punchcardPreferences, 259 - IsTnglSh: s.Config.Pds.IsTnglShUser(user.Pds()), 276 + IsTnglSh: isTnglSh, 260 277 IsDeactivated: isDeactivated, 261 278 HandleOpen: r.URL.Query().Get("handle") == "1", 262 279 }) ··· 716 733 717 734 func (s *Settings) elevateForHandle(w http.ResponseWriter, r *http.Request) { 718 735 user := s.OAuth.GetMultiAccountUser(r) 719 - if !s.Config.Pds.IsTnglShUser(user.Pds()) { 736 + if isTngl, _ := s.isTnglShUser(r.Context(), syntax.DID(user.Active.Did)); !isTngl { 720 737 http.Redirect(w, r, "/settings/profile", http.StatusSeeOther) 721 738 return 722 739 } ··· 744 761 745 762 func (s *Settings) updateHandle(w http.ResponseWriter, r *http.Request) { 746 763 user := s.OAuth.GetMultiAccountUser(r) 747 - if !s.Config.Pds.IsTnglShUser(user.Pds()) { 764 + if isTngl, _ := s.isTnglShUser(r.Context(), syntax.DID(user.Active.Did)); !isTngl { 748 765 s.Pages.Notice(w, "handle-error", "Handle changes are only available for tngl.sh accounts.") 749 766 return 750 767 }

+7 -6

appview/state/router.go

··· 211 211 212 212 func (s *State) SettingsRouter() http.Handler { 213 213 settings := &settings.Settings{ 214 - Db: s.db, 215 - OAuth: s.oauth, 216 - Pages: s.pages, 217 - Config: s.config, 218 - CfClient: s.cfClient, 219 - Logger: log.SubLogger(s.logger, "settings"), 214 + Db: s.db, 215 + OAuth: s.oauth, 216 + Pages: s.pages, 217 + Config: s.config, 218 + CfClient: s.cfClient, 219 + Logger: log.SubLogger(s.logger, "settings"), 220 + IdResolver: s.idResolver, 220 221 } 221 222 222 223 return settings.Router()