Skip to content

Endpoint user/challenger/recover #636

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
avivash wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Endpoint user/challenger/recover #636

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b946edd
Feat: first bit of user/challenge/recover
avivash Feb 25, 2023
890e57b
Chore: return challenge
avivash Mar 2, 2023
222db1f
Chore: enable auth header check in user/challenge/recover
avivash Mar 6, 2023
db24e2c
Fix: remove unused imports
avivash Mar 7, 2023
3265416
Fix: capture username from endpoint and add to docs
avivash Mar 22, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions fission-web-api/library/Fission/Web/API/User/Challenge/Types.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
module Fission.Web.API.User.Challenge.Types (Routes (..)) where

import Fission.Challenge.Types

import Fission.Web.API.Prelude

import Fission.User.Username.Types

import qualified Fission.Web.API.Auth.Types as Auth

data Routes mode = Routes
{ recover ::
mode
:- "recover"
:> Summary "Return challenge for account recovery"
--
:> Capture "Username" Username
--
:> Auth.HigherOrder
Copy link
Member

@matheus23 matheus23 Mar 7, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line ensures that only fission-registered accounts (and UCANs that are delegated from fission-registered accounts) can make these requests 👍

:> Post '[JSON] Challenge
}
deriving Generic
12 changes: 7 additions & 5 deletions fission-web-api/library/Fission/Web/API/User/Types.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import qualified Fission.Web.API.Relay.Types as Relay
import qualified Fission.Web.API.User.Create.Types as Create
import qualified Fission.Web.API.User.DID.Types as DID
import qualified Fission.Web.API.User.DataRoot.Types as DataRoot
import qualified Fission.Web.API.User.Challenge.Types as Challenge
import qualified Fission.Web.API.User.Email.Types as Email
import qualified Fission.Web.API.User.ExchangeKey.Types as ExchangeKeys
import qualified Fission.Web.API.User.Password.Reset.Types as Password
Expand All @@ -18,11 +19,12 @@ data RoutesV3 mode = RoutesV3
deriving Generic

data RoutesV2 mode = RoutesV2
{ dataRoot :: mode :- "data" :> ToServantApi DataRoot.RoutesV2
, email :: mode :- "email" :> ToServantApi Email.Routes
, did :: mode :- "did" :> ToServantApi DID.RoutesV_
, whoAmI :: mode :- "whoami" :> ToServantApi WhoAmI.Routes
, linkingRelay :: mode :- "link" :> ToServantApi Relay.Routes
{ dataRoot :: mode :- "data" :> ToServantApi DataRoot.RoutesV2
, challenge :: mode :- "challenge" :> ToServantApi Challenge.Routes
, email :: mode :- "email" :> ToServantApi Email.Routes
, did :: mode :- "did" :> ToServantApi DID.RoutesV_
, whoAmI :: mode :- "whoami" :> ToServantApi WhoAmI.Routes
, linkingRelay :: mode :- "link" :> ToServantApi Relay.Routes
, create :: mode :- Create.WithDID
}
deriving Generic
Expand Down
8 changes: 8 additions & 0 deletions fission-web-client/library/Fission/Web/Client/V2.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ module Fission.Web.Client.V2
, setDIDViaUCAN
, setDIDViaChallenge
--
, recoverViaChallenge
--
, verifyViaEmail
, resendVerificationEmail
, recoverViaEmail
Expand All @@ -36,6 +38,7 @@ import qualified Fission.Web.API.Types as Fission

import qualified Fission.Web.API.User.DID.Types as User.DID
import qualified Fission.Web.API.User.DataRoot.Types as User.DataRoot
import qualified Fission.Web.API.User.Challenge.Types as User.Challenge
import qualified Fission.Web.API.User.Email.Types as User.Email
import qualified Fission.Web.API.User.Types as User
import qualified Fission.Web.API.User.WhoAmI.Types as User.WhoAmI
Expand Down Expand Up @@ -83,6 +86,11 @@ Fission.Routes
, setViaChallenge = setDIDViaChallenge
}

, challenge = fromServant @_ @(AsClientT ClientM) ->
User.Challenge.Routes
{ recover = recoverViaChallenge
}

, email = fromServant @_ @(AsClientT ClientM) ->
User.Email.Routes
{ verify = verifyViaEmail
Expand Down
2 changes: 2 additions & 0 deletions fission-web-server/library/Fission/Web/Server/Handler/User.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import qualified Fission.Web.Server.Handler.Relay as Relay
import qualified Fission.Web.Server.Handler.User.Create as Create
import qualified Fission.Web.Server.Handler.User.DID as DID
import qualified Fission.Web.Server.Handler.User.DataRoot as DataRoot
import qualified Fission.Web.Server.Handler.User.Challenge as Challenge
import qualified Fission.Web.Server.Handler.User.Email as Email
import qualified Fission.Web.Server.Handler.User.ExchangeKey as ExchangeKey
import qualified Fission.Web.Server.Handler.User.Password.Reset as Password.Reset
Expand Down Expand Up @@ -69,6 +70,7 @@ handlerV2 =
User.RoutesV2
{ create = Create.withDID
, whoAmI = genericServerT WhoAmI.handler
, challenge = genericServerT Challenge.handler
, email = genericServerT Email.handler
, did = genericServerT DID.handlerV_
, linkingRelay = genericServerT Relay.handler
Expand Down
34 changes: 34 additions & 0 deletions fission-web-server/library/Fission/Web/Server/Handler/User/Challenge.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
module Fission.Web.Server.Handler.User.Challenge (handler) where

import Servant
import Servant.Server.Generic

import Fission.Prelude

import qualified Fission.Web.API.User.Challenge.Types as Challenge

import Fission.Web.Server.Authorization.Types
import qualified Fission.Web.Server.Error as Web.Err
import Fission.Web.Server.Models
import qualified Fission.Web.Server.RecoveryChallenge.Creator.Class as RecoveryChallenge
import Fission.Web.Server.User.Retriever.Class as User

handler ::
( RecoveryChallenge.Creator m
, User.Retriever m
, MonadThrow m
, MonadLogger m
, MonadTime m
)
=> Challenge.Routes (AsServerT m)
handler = Challenge.Routes {..}
where
recover username Authorization { about = Entity userId _ } = do
Entity _ User { } <- Web.Err.ensureMaybe couldntFindUser =<< getByUsername username
now <- currentTime
challenge <- RecoveryChallenge.create userId now
Copy link
Member

@matheus23 matheus23 Mar 7, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

However, here we don't check any other property about the authentication, right?

So if I read this correctly, doesn't this allow any fission user to issue & get a challenge for any other fission user?

E.g. a malicious actor could create a new fission account, and then use their UCAN to run a request /user/challenge/recover/bob, and then use the response to call /user/did/bob/{challenge} with a DID they control as the body for that request, essentially taking away my control over bob's fission account and giving it to themselves?

Copy link
Member Author

@avivash avivash Mar 7, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh thanks for catching that! I'll admit I have a fairly tenuous understanding of this codebase. This new endpoint was cobbled together from bits of various other ones I saw in the repo 😅 Looking at some other examples, I think I see how to add that validation here. I'll get this fixed today!

Copy link
Member

@matheus23 matheus23 Mar 7, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the goal, actually? How should the auth gating work for that endpoint?
Is the plan to require the user to be authed to access this endpoint? Although in that case I wonder why they'd need to recover in the first place.
Is the idea to allow this endpoint only for the email service? I.e. the email service has a special DID that this fission server respects and it opens up this endpoint for it?
Or is the idea that the fission service's DID would be the "root owner" in this case, and we'd check that the root DID of a UCAN coming in to use the recovery endpoint is the fission service's DID, allowing us to delegate a UCAN for accessing the recovery endpoint to other services such as the email service?

Copy link
Member Author

@avivash avivash Mar 7, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current approach we're taking is:

  • On registration, the client builds a UCAN that delegates root access to the rs-email-service's DID
  • That UCAN is sent to the rs-email-service and stored in a DB that maps usernames to UCANS
  • Then, when a user goes to recover their account, they enter their username and a request is sent from the client to the rs-email-service, which finds the associated UCAN in the DB and uses that to make a request to this new fission server endpoint to get a challenge
  • With the challenge in hand, the rs-email-service should be able to call /v2/api/user/did/{Username}/{Challenge} to recover the user's account

Copy link
Member Author

@avivash avivash Mar 7, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I should add that the rs-email-service is just meant to be an example of how this email recovery flow could work. We expect devs to create their own services that maybe reference our rs-email-service example

Copy link
Member

@matheus23 matheus23 Mar 7, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That sound awesome and exactly like what I'd expect.

Let's go 🚀

return challenge

where
couldntFindUser =
err422 { errBody = "Couldn't find a user with this username" }
4 changes: 2 additions & 2 deletions fission-web-server/package.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
name: fission-web-server
version: "2.21.0.0"
version: "2.21.0.1"
category: API
author:
- Brooklyn Zelenka
Expand All @@ -15,7 +15,7 @@ maintainer:
- [email protected]
- [email protected]
- [email protected]
copyright: © 2021 Fission Internet Software Services for Open Networks Inc.
copyright: © 2023 Fission Internet Software Services for Open Networks Inc.
license: AGPL-3.0-or-later
license-file: LICENSE
github: fission-suite/fission
Expand Down