Skip to content
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

Jump to bottom

feat: device account version 3 #2741

Closed
wants to merge 3 commits into from
Closed

feat: device account version 3 #2741

wants to merge 3 commits into from

Conversation

ntheile
Copy link
Contributor

@ntheile ntheile commented Jun 12, 2023 •
edited
Loading

Device Account v3

3 flows

(1) New User/Password Flow

  1. get app-check jwt add to header X-Appcheck-Token
  2. mobile app generates locally a uuidV4UserName and uuidV4password
  3. REST
POST `/auth/create/device-account`
Authorization: basic base64(username:password) # using basic auth so telemetry does not log
X-DeviceId: deviceId # from react-native-device-info?
  1. oathkeeper decision api to verify appcheck jwt via rule
  • kratos will create identity with and uuidV4Username and uuidV4Password
    • In our backend (transient_payload?) store DeviceId in mongo user collection,
  • RETURN kratos session token
    • mobile app needs to backup uuidV4Username and uuidV4Password to keychain

(2) Login User/Password Flow

  1. get app-check jwt add to header X-Appcheck-Token
  2. retrieve uuidV4Username and uuidV4Password from apollo client storage (or keychain if uninstalled)
  3. REST
POST /auth/login/device-account
Authorization:  basic base64(username:password)

RETURN kratos session token

oathkeeper might be able to validate basic auth in a future PR here ory/oathkeeper#1044

(3) Upgrade To Phone Flow

  1. captcha flow in mobile app
  2. grapqhl
Post /graphql MUTATION UserLoginUpgrade (or REST /auth/upgrade/phone ??)
Authorization: bearer kratosSessionToken
BODY: phone + code
  1. backend
  isValid(code) then upgrade
    a) kratos add phone trait
    b) change scheme from "username_password_deviceid_v0" to "phone_no_pasword_v0"
    c) change password from uuidV4password to KratosMasterPassword

RETURN success=true|error and optional kratos session token (if phone account already exists)

@ntheile ntheile changed the title Feat device account version 3 feat: device account version 3 Jun 12, 2023
github-advanced-security[bot]
github-advanced-security bot found potential problems Jun 12, 2023
View reviewed changes
test/e2e/servers/ory/oathkeeper.spec.ts
if (res instanceof Error) throw res

const decoded = decode(res, { complete: true })
expect(decoded?.payload?.sub).toBe("anon")
})

it("error if an invalid token is provided", async () => {
const res = await sendOathkeeperRequest("invalid.token" as SessionToken)
const res = await sendOathkeeperRequest("invalid.token" as SessionToken, "graphql")

Check failure

Code scanning / CodeQL

Hard-coded credentials

The hard-coded value "invalid.token" is used as [authorization header](1).
test/e2e/servers/ory/oathkeeper.spec.ts
if (res instanceof Error) throw res

const decoded = decode(res, { complete: true })
expect(decoded?.payload?.sub).toBe("anon")
})

it("error if an invalid token is provided", async () => {
const res = await sendOathkeeperRequest("invalid.token" as SessionToken)
const res = await sendOathkeeperRequest("invalid.token" as SessionToken, "graphql")

Check failure

Code scanning / CodeQL

Hard-coded credentials

The hard-coded value "invalid.token" is used as [authorization header](1).
test/e2e/servers/ory/oathkeeper.spec.ts
it("return sub if jwt is provided", async () => {
// TODO jwt from helper
const token =
"eyJhbGciOiJSUzI1NiIsImtpZCI6IjFiOTdiMjIxLWNhMDgtNGViMi05ZDA5LWE1NzcwZmNjZWIzNyJ9.eyJzdWIiOiIxOjcyMjc5Mjk3MzY2OmFuZHJvaWQ6VEVTVEUyRUFDQ09VTlQ1YWE3NWFmNyIsImF1ZCI6WyJwcm9qZWN0cy83MjI3OTI5NzM2NiIsInByb2plY3RzL2dhbG95YXBwIl0sInByb3ZpZGVyIjoiZGVidWciLCJpc3MiOiJodHRwczovL2ZpcmViYXNlYXBwY2hlY2suZ29vZ2xlYXBpcy5jb20vNzIyNzkyOTczNjYiLCJleHAiOjI2MzkwMDAwNjl9.Fh11HcuTal_S_26xFwIUWYivY0NzKGYrpBwNgQ-1QnfLZwUaHlMCX4hj4tcRJiKMX2UU_pnZCWgVnBqM9rbeSLFj35OvyP0z4rnflLOOl-UBrQQs4pVSUCpmh8eLX5lkh27KhdGOifND3jJPkKhPeVI9-hpZKNTYdU9y3M1yFF4BjvHs05nf8Zu3tWfpj0_LNPE-H0eXiiHaEUDv_GPA4HgLSAyxdh8bFoVC36UjpG-vm8Tt7jOUDnGc3s7jQk_lIJ3uCs8JXU4LfhSAQS6Q9UYmpFFUgsrUaZ6T_o2XTZtHgd_9qOUVvTChL-0dDGyDvB1tzofwIzLwxj7TGoEDGQ" as SessionToken

Check failure

Code scanning / CodeQL

Hard-coded credentials

The hard-coded value "eyJhbGciOiJSUzI1NiIsImtpZCI6IjFiOTdiMjIxLWNhMDgtNGViMi05ZDA5LWE1NzcwZmNjZWIzNyJ9.eyJzdWIiOiIxOjcyMjc5Mjk3MzY2OmFuZHJvaWQ6VEVTVEUyRUFDQ09VTlQ1YWE3NWFmNyIsImF1ZCI6WyJwcm9qZWN0cy83MjI3OTI5NzM2NiIsInByb2plY3RzL2dhbG95YXBwIl0sInByb3ZpZGVyIjoiZGVidWciLCJpc3MiOiJodHRwczovL2ZpcmViYXNlYXBwY2hlY2suZ29vZ2xlYXBpcy5jb20vNzIyNzkyOTczNjYiLCJleHAiOjI2MzkwMDAwNjl9.Fh11HcuTal_S_26xFwIUWYivY0NzKGYrpBwNgQ-1QnfLZwUaHlMCX4hj4tcRJiKMX2UU_pnZCWgVnBqM9rbeSLFj35OvyP0z4rnflLOOl-UBrQQs4pVSUCpmh8eLX5lkh27KhdGOifND3jJPkKhPeVI9-hpZKNTYdU9y3M1yFF4BjvHs05nf8Zu3tWfpj0_LNPE-H0eXiiHaEUDv_GPA4HgLSAyxdh8bFoVC36UjpG-vm8Tt7jOUDnGc3s7jQk_lIJ3uCs8JXU4LfhSAQS6Q9UYmpFFUgsrUaZ6T_o2XTZtHgd_9qOUVvTChL-0dDGyDvB1tzofwIzLwxj7TGoEDGQ" is used as [authorization header](1).
test/e2e/servers/ory/oathkeeper.spec.ts
it("return sub if jwt is provided", async () => {
// TODO jwt from helper
const token =
"eyJhbGciOiJSUzI1NiIsImtpZCI6IjFiOTdiMjIxLWNhMDgtNGViMi05ZDA5LWE1NzcwZmNjZWIzNyJ9.eyJzdWIiOiIxOjcyMjc5Mjk3MzY2OmFuZHJvaWQ6VEVTVEUyRUFDQ09VTlQ1YWE3NWFmNyIsImF1ZCI6WyJwcm9qZWN0cy83MjI3OTI5NzM2NiIsInByb2plY3RzL2dhbG95YXBwIl0sInByb3ZpZGVyIjoiZGVidWciLCJpc3MiOiJodHRwczovL2ZpcmViYXNlYXBwY2hlY2suZ29vZ2xlYXBpcy5jb20vNzIyNzkyOTczNjYiLCJleHAiOjI2MzkwMDAwNjl9.Fh11HcuTal_S_26xFwIUWYivY0NzKGYrpBwNgQ-1QnfLZwUaHlMCX4hj4tcRJiKMX2UU_pnZCWgVnBqM9rbeSLFj35OvyP0z4rnflLOOl-UBrQQs4pVSUCpmh8eLX5lkh27KhdGOifND3jJPkKhPeVI9-hpZKNTYdU9y3M1yFF4BjvHs05nf8Zu3tWfpj0_LNPE-H0eXiiHaEUDv_GPA4HgLSAyxdh8bFoVC36UjpG-vm8Tt7jOUDnGc3s7jQk_lIJ3uCs8JXU4LfhSAQS6Q9UYmpFFUgsrUaZ6T_o2XTZtHgd_9qOUVvTChL-0dDGyDvB1tzofwIzLwxj7TGoEDGQ" as SessionToken

Check failure

Code scanning / CodeQL

Hard-coded credentials

The hard-coded value "eyJhbGciOiJSUzI1NiIsImtpZCI6IjFiOTdiMjIxLWNhMDgtNGViMi05ZDA5LWE1NzcwZmNjZWIzNyJ9.eyJzdWIiOiIxOjcyMjc5Mjk3MzY2OmFuZHJvaWQ6VEVTVEUyRUFDQ09VTlQ1YWE3NWFmNyIsImF1ZCI6WyJwcm9qZWN0cy83MjI3OTI5NzM2NiIsInByb2plY3RzL2dhbG95YXBwIl0sInByb3ZpZGVyIjoiZGVidWciLCJpc3MiOiJodHRwczovL2ZpcmViYXNlYXBwY2hlY2suZ29vZ2xlYXBpcy5jb20vNzIyNzkyOTczNjYiLCJleHAiOjI2MzkwMDAwNjl9.Fh11HcuTal_S_26xFwIUWYivY0NzKGYrpBwNgQ-1QnfLZwUaHlMCX4hj4tcRJiKMX2UU_pnZCWgVnBqM9rbeSLFj35OvyP0z4rnflLOOl-UBrQQs4pVSUCpmh8eLX5lkh27KhdGOifND3jJPkKhPeVI9-hpZKNTYdU9y3M1yFF4BjvHs05nf8Zu3tWfpj0_LNPE-H0eXiiHaEUDv_GPA4HgLSAyxdh8bFoVC36UjpG-vm8Tt7jOUDnGc3s7jQk_lIJ3uCs8JXU4LfhSAQS6Q9UYmpFFUgsrUaZ6T_o2XTZtHgd_9qOUVvTChL-0dDGyDvB1tzofwIzLwxj7TGoEDGQ" is used as [authorization header](1).
github-advanced-security[bot]
github-advanced-security bot found potential problems Jun 13, 2023
View reviewed changes
src/servers/middlewares/auth-router.ts
Comment on lines +203 to +222
authRouter.post("/create/device-account", async (req, res) => {
try {
//const { deviceId, uuidV4Username, uuidV4Password } = req.body
const user = basicAuth(req)
if (user?.name && user?.pass) {
const username = user.name
const password = user.pass
console.log("username", username)
console.log("password", password)
// TODO call create device account
return res.status(200).send({
result: "NOT IMPLEMENTED",
})
} else {
return res.status(401).send({ result: "Unauthorized" })
}
} catch (e) {
return res.status(500).send({ result: "" })
}
})

Check failure

Code scanning / CodeQL

Missing rate limiting

This route handler performs [authorization](1), but is not rate-limited.
@UncleSamtoshi
Copy link
Contributor

UncleSamtoshi commented Jun 13, 2023 •
edited
Loading

A couple questions:

  • When a user upgrades their account do they get a new kratos session token? Additionally, is their old username deleted so that it could be reused, or do i need to generate a new username+password in mobile?
  • What happens with the deviceAccount sessionToken, is it still valid for something?
  • Could we have the same mutation for create account and login similar to what we do for phone authentication? If we are reusing the same username and password for the device account, I wont be able to tell if the account has been created already.

@nicolasburtey
Copy link
Member

superseeded by #2747

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ntheile @UncleSamtoshi @nicolasburtey