Create RFC for Identities #57

stnguyen90 · 2023-05-16T00:05:36Z

Rendered: https://github.com/appwrite/rfc/blob/feat-023-connected-accounts/023-identities/README.md

What does this PR do?

Proposal for separating OAuth2 from Sessions

Related PRs and Issues

TBD

Have you read the Contributing Guidelines on issues?

Yes

eldadfux

Steven, great work, super interesting feature. I left a few comments, thoughts, and questions.

eldadfux · 2023-06-24T09:16:37Z

023-connected-accounts/README.md

+
+##### List Connected Accounts
+
+**GET /v1/account/connected**


Connected accounts is very misleading. When I first heard of it I thought it was about connecting 2 existing Appwrite accounts and allowing to switch between them. This features is actually just refactoring of how we handle OAuth2.

eldadfux · 2023-06-24T09:21:00Z

023-connected-accounts/README.md

+
+##### List Connected Accounts
+
+**GET /v1/account/connected**


Suggested change

**GET /v1/account/connected**

**GET /v1/account/oauth2/connections**

Connected accounts is very misleading. When I first heard of it I thought it was about connecting 2 existing Appwrite accounts and allowing to switch between them. This features is actually just refactoring of how we handle OAuth2. Also, the connected is a past tense verb where a REST API should be represented by nouns.

Wouldn't it be useful to make this flexible so that we could support other connections? Maybe multiple phone numbers or linking multiple emails? Merging Appwrite users can also be helpful.

eldadfux · 2023-06-24T09:24:53Z

023-connected-accounts/README.md

+
+##### Get a Connected Account
+
+**GET /v1/account/connected/{id}**


Suggested change

**GET /v1/account/connected/{id}**

**GET /v1/account/oauth/connections/{id}**

eldadfux · 2023-06-24T09:29:20Z

023-connected-accounts/README.md

+
+##### Connecting to an OAuth2 provider
+
+1. User makes authenticated API call to `GET /v1/account/sessions/oauth2/{provider}`


Is it safe to assume that the main difference here from the existing flow is that if a user is already logged in with Appwrite when creating an OAuth session the OAuth connection will be attached to their current account regardless of the email?

Yes, during the /redirect.

eldadfux · 2023-06-24T09:30:26Z

023-connected-accounts/README.md

+If possible, explain what actions we can take to avoid that.
+-->
+
+The existing OAuth2 APIs will still work as they do now. The only difference is a connected account will be created if one does not exist.


This is a better place in the RFC to discuss data migrations due to changes in data structure.

Added a line.

eldadfux · 2023-06-24T09:31:27Z

023-connected-accounts/README.md

+
+<!-- Write your answer below. -->
+
+1. How to connect an account on mobile?


What is the main challenge? Passing the current use session token to the webview?

If yes, we could consider another form of short living token with limited access rights. We have similar problems with file previews and other resources as well.

eldadfux · 2023-06-24T09:32:07Z

023-connected-accounts/README.md

+<!-- Write your answer below. -->
+
+1. How to connect an account on mobile?
+2. Should we support a way to get all connected accounts from a particular provider?


We can do this using the list endpoint queries.

eldadfux · 2023-06-24T09:33:29Z

023-connected-accounts/README.md

+
+1. How to connect an account on mobile?
+2. Should we support a way to get all connected accounts from a particular provider?
+3. What to do after a connected accounts refresh token no longer works?


I would expect maybe something 401, which can notify the developer they need to re-authenticate the OAuth session to obtain a new access token. How often does this happen today for the different providers? how long would an average access token last before expiration?

I would expect maybe something 401

You mean if the user is using an OAuth2 session? This means we would need to check the refresh token on each request or something.

In addition, we may need to check the tokens on each list connected accounts API call so that we properly return the correct status.

eldadfux · 2023-06-24T09:35:08Z

023-connected-accounts/README.md

+1. How to connect an account on mobile?
+2. Should we support a way to get all connected accounts from a particular provider?
+3. What to do after a connected accounts refresh token no longer works?
+4. What should happen to related sessions when a connected account is deleted?


I would leave this unattached. If dev wish to take action here, they can delete relevant sessions with existing APIs. I wouldn't be opinionated and decide that deleting an account is necessarily connected with a security concern, this is not always the case.

eldadfux · 2023-06-24T09:37:51Z

023-connected-accounts/README.md

+2. Should we support a way to get all connected accounts from a particular provider?
+3. What to do after a connected accounts refresh token no longer works?
+4. What should happen to related sessions when a connected account is deleted?
+5. What should happen to a connected account when the related session is deleted?


Nothing. You can logout, it doesn't imply you want your connection removed. If developers want for any reason to tie the two together they can do it using the two relevant APIs.

Allow users to connect multiple accounts to their Appwrite account regardless of the email address in the external system.

stnguyen90 requested a review from TorstenDittmann May 16, 2023 00:05

stnguyen90 force-pushed the feat-023-connected-accounts branch 3 times, most recently from 645773c to a0a15fe Compare May 17, 2023 17:45

stnguyen90 mentioned this pull request May 18, 2023

Separate OAuth2 data out of Sessions and into User Identities appwrite/appwrite#5551

Closed

2 tasks

stnguyen90 force-pushed the feat-023-connected-accounts branch from a0a15fe to b51e46b Compare May 24, 2023 15:50

stnguyen90 marked this pull request as ready for review June 23, 2023 12:38

eldadfux requested changes Jun 24, 2023

View reviewed changes

stnguyen90 force-pushed the feat-023-connected-accounts branch from b51e46b to d82c2b2 Compare June 29, 2023 02:23

stnguyen90 requested a review from eldadfux June 29, 2023 02:24

stnguyen90 force-pushed the feat-023-connected-accounts branch from d82c2b2 to 19579b8 Compare July 13, 2023 23:26

Create RFC for identities

400229a

Allow users to connect multiple accounts to their Appwrite account regardless of the email address in the external system.

stnguyen90 force-pushed the feat-023-connected-accounts branch from 19579b8 to 400229a Compare July 13, 2023 23:47

stnguyen90 changed the title ~~DRAFT: Create RFC for Connected Accounts~~ DRAFT: Create RFC for Identities Jul 13, 2023

stnguyen90 changed the title ~~DRAFT: Create RFC for Identities~~ Create RFC for Identities Jul 20, 2023

stnguyen90 marked this pull request as draft October 5, 2023 17:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create RFC for Identities #57

Create RFC for Identities #57

stnguyen90 commented May 16, 2023 •

edited

Loading

eldadfux left a comment

eldadfux Jun 24, 2023

eldadfux Jun 24, 2023

stnguyen90 Jun 28, 2023

eldadfux Jun 24, 2023

eldadfux Jun 24, 2023

stnguyen90 Jun 28, 2023

eldadfux Jun 24, 2023

stnguyen90 Jun 29, 2023

eldadfux Jun 24, 2023

eldadfux Jun 24, 2023

eldadfux Jun 24, 2023

eldadfux Jun 24, 2023

stnguyen90 Jun 29, 2023

eldadfux Jun 24, 2023

eldadfux Jun 24, 2023

	GET /v1/account/connected
	GET /v1/account/oauth2/connections


		##### Get a Connected Account

		GET /v1/account/connected/{id}

	GET /v1/account/connected/{id}
	GET /v1/account/oauth/connections/{id}


		##### Connecting to an OAuth2 provider

		1. User makes authenticated API call to `GET /v1/account/sessions/oauth2/{provider}`


		<!-- Write your answer below. -->

		1. How to connect an account on mobile?

Create RFC for Identities #57

Are you sure you want to change the base?

Create RFC for Identities #57

Conversation

stnguyen90 commented May 16, 2023 • edited Loading

What does this PR do?

Related PRs and Issues

Have you read the Contributing Guidelines on issues?

eldadfux left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stnguyen90 commented May 16, 2023 •

edited

Loading