Skip to content
This repository has been archived by the owner on Sep 29, 2023. It is now read-only.

Split auth guide, add label, team, and password security to docs. #412

Merged
merged 23 commits into from
Aug 27, 2023

Conversation

gewenyu99
Copy link
Contributor

What does this PR do?

(Provide a description of what this PR does.)

Test Plan

(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work.)

Related PRs and Issues

(If this PR is related to any other PR or resolves any issue or related to any issue link all related PR and issues here.)

Have you read the Contributing Guidelines on issues?

(Write your answer here.)

@stnguyen90 stnguyen90 self-requested a review August 14, 2023 18:10
@gewenyu99 gewenyu99 marked this pull request as ready for review August 17, 2023 20:37

[TODO @steven: TEAMS docs]

[TODO @steven: Labels docs]
Copy link
Contributor

@stnguyen90 stnguyen90 Aug 17, 2023

Choose a reason for hiding this comment

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

Labels are a good way to flag a user to grant them access to resources. For example, a subscriber label can be added to a user once they've purchased a subscription:

const sdk = require('node-appwrite');

const client = new sdk.Client();

const users = new sdk.Users(client);

users.updateLabels(
  '[USER_ID]',
  [
    sdk.Role.label('subscriber'),
  ]
);

And a collection can be be given read access to users with that label:

const sdk = require('node-appwrite');

const client = new sdk.Client();

const databases = new sdk.Databases(client);

databases.updateCollection(
  '[DATABASE_ID]',
  '[COLLECTION_ID]',
  '[NAME]',
  [
    sdk.Permissions.read(sdk.Role.label('subscriber')),
  ]
);

Copy link
Contributor

@stnguyen90 stnguyen90 left a comment

Choose a reason for hiding this comment

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

I added some notes. Let me know if you need any additional clarification.

Each user can also have their own preference object, which you can use to save preferences such as theme, language, and notification settings.
</p>

[TODO @steven: TEAMS docs]
Copy link
Contributor

Choose a reason for hiding this comment

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

Teams are a good way to allow users to collaborate and share access to resources. For example, in a todo app, a user can create a team for one of their todo lists and invite another user to the team to grant the other user access. The invited user can accept the invitation to gain access or ignore it if they don't want to gain access.

If you'd like for users to collaborate without requiring users to accept the invitation, use an Appwrite Function and server SDK to add a user to a team.


<h2><a href="#profile" id="profile">OAuth 2 Profile</a></h2>
<p>
[TODO: @steven fill the docs to access oauth account profile information]
Copy link
Contributor

Choose a reason for hiding this comment

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

It's common to fetch information from an OAuth2 provider such as an avatar image. To do this you can use the access token from the OAuth2 provider and make API calls to the provider.

After creating an OAuth2 session, you can fetch the session with:

import { Client, Account } from "appwrite";

const client = new Client();

const account = new Account(client);

account.getSession('current');

An OAuth2 session will have the following attributes set:

  • provider: The OAuth2 Provider
  • providerUid: User ID from the OAuth2 Provider
  • provderAccessToken: Access token from the OAuth2 provider that can be used to make API calls to the OAuth2 Provider

Refer to the docs for the OAuth2 provider for how to make an API call with the access token.

@gewenyu99 gewenyu99 changed the base branch from main to 1.4.x August 17, 2023 22:38
@gewenyu99
Copy link
Contributor Author

localhost_2080_docs_authentication

@gewenyu99
Copy link
Contributor Author

gewenyu99 commented Aug 18, 2023

localhost_2080_docs_authentication-email-pass

@gewenyu99
Copy link
Contributor Author

localhost_2080_docs_authentication (2)

@gewenyu99
Copy link
Contributor Author

localhost_2080_docs_authentication (3)

@gewenyu99
Copy link
Contributor Author

localhost_2080_docs_authentication (4)

@gewenyu99
Copy link
Contributor Author

localhost_2080_docs_authentication (5)

@gewenyu99
Copy link
Contributor Author

localhost_2080_docs_authentication (6)

@gewenyu99
Copy link
Contributor Author

localhost_2080_docs_authentication (6)

@gewenyu99
Copy link
Contributor Author

localhost_2080_docs_authentication (7)

@eldadfux
Copy link
Member

  • In the console, we call personal information -> personal data, let's be consistent.
  • We should probably mention that for non-web platforms success and failure URLs are optional and Appwrite will handle the redirect. They also need some additional settings to allow the redirect to their apps to work.
  • Is magic URL working on mobile? I think we also need some additional settings there

@gewenyu99
Copy link
Contributor Author

  • In the console, we call personal information -> personal data, let's be consistent.
  • We should probably mention that for non-web platforms success and failure URLs are optional and Appwrite will handle the redirect. They also need some additional settings to allow the redirect to their apps to work.
  • Is magic URL working on mobile? I think we also need some additional settings there

I was considering just dropping magic URL in the docs... I don't think it works.

<h2><a href="#attach-account" id="attach-account">Attaching an Account</a></h2>
<p>
Anonymous users cannot sign back in.
If they close their browser, or go to another computer, they won't be able to log in again.
Copy link
Contributor

Choose a reason for hiding this comment

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

Step of closing browser here sounds irelevant. Also would be good to mention here that we store session information in cookies, so clearing browser might also cause sign out.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ye


<p>
Initialize the log in process with the <a href="/docs/client/account#accountCreateMagicURLSession">Create Magic URL Session</a> route.
If the email has never been used, a <b>new user ID is generated</b>, then the user will receive an email.
Copy link
Contributor

Choose a reason for hiding this comment

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

new user ID is generated
Buuut only if they pass unique(). If they provide custom ID, user with that ID will try to be created (unless its already used by other user)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Comment on lines 142 to 143
userId: 'email@example.com',
secret: '[SECRET]',
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
userId: 'email@example.com',
secret: '[SECRET]',
userId: '[ID_FROM_EMAL_PARAMS]',
secret: '[SECRET_FROM_EMAIL_PARAMS]',

Let's do something like that here, and in other exmapes where we don't provide example of parsing URL

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We just removed mobile platform... MagicURL doesn't work with out some voodoo magic. I saw Damodar's attempt of explaining it. It's a little much.


const account = new Account(client);

const user = await account.updatePrefs({darkTheme: true, language: 'en'});</code></pre>
Copy link
Contributor

Choose a reason for hiding this comment

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

Lets use typical promise.then() syntax

Copy link
Contributor

Choose a reason for hiding this comment

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

Please check the whole PR. This should be done in ~dozen palces.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Some places, we can't use promise.then(), basically when ever we have a 2 part example where you need to save something from the returned object.

But this should indeed be .then

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed here. There are some palces where this is used on purpose, because we're saving the objects returned.

Comment on lines 85 to 101
accountUpdatePrefs(
prefs: "{\"darkTheme\": true, \"language\": \"en\"}"
) {
_id
_createdAt
_updatedAt
name
registration
status
passwordUpdate
email
phone
emailVerification
phoneVerification
prefs {
data
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can remove most of this. getting just prefs back would be fine I think

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

<div class="ide margin-top-small" data-lang="nodejs" data-lang-label="Node.js SDK">
<pre class="line-numbers"><code class="prism language-javascript" data-prism>const sdk = require('node-appwrite');

const client = new sdk.Client();
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing client initialization. setEndpoint, setPRoject, setApiKey..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Comment on lines +227 to +228
.setProject('5df5acd0d48c2') // Your project ID
.setKey('919c2d18fb5d4...a2ae413da83346ad2') // Your secret API key
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can use typical placeholders here

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's check all places

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We use '919c2d18fb5d4...a2ae413da83346ad2' all over the place, I think it's fine. We historically found that people didn't know that a key looks like a giant hash


<p>
You'll be redirected to the OAuth 2 provider's login page to log in.
Once complete, you'll be redirected to the redirect URL configured in your OAuth 2 provider.
Copy link
Contributor

Choose a reason for hiding this comment

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

This is internal complexity (it redirect to appwrite API)
We could say:

Once complete, you will be redirected back to Appwrite API where user and session is created. User is next (almost instantly) redirected back to your application.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

</p>

<p>
You can optionally configure <code>success</code> or <code>failure</code> redirect links on web to handle success and failure scenarios.
Copy link
Contributor

Choose a reason for hiding this comment

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

For web, let's talk about 1st party cookies.

By default, OAuth flow on web won't work in some browsers (most mobile browsers, and Safari). Reason is they don't allow your web app (on your domain) to access cookies set on API cloud.appwrite.io domain.
To solve this, you need to setup custom domain that is subdomain of your app domain, for example appwrite.myapp.com

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, this is just a "bug" we need to fix. I discuss this many times with Eldad, the consesus is we just need to fix this. This is way too much for someone to consider...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

😆 Yeah... this will need to be covered in depth later but what's more likely is that we'll want to fix this behavior...

Gonna omit it this time round.

<h2><a href="#refresh" id="refresh">Refresh Tokens</a></h2>
<p>
OAuth 2 sessions expire to protect from security risks.
OAuth 2 sessions should be periodically refreshed to keep the user authenticated.
Copy link
Contributor

@Meldiron Meldiron Aug 27, 2023

Choose a reason for hiding this comment

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

Suggested change
OAuth 2 sessions should be periodically refreshed to keep the user authenticated.
OAuth 2 sessions should be refreshed before using an access token that is expired. Check value of <code>providerAccessTokenExpiry</code> to know if the token is expired. Refreshing before every request might cause rate limit problems.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added, but different from this.

<td> User ID from the OAuth 2 Provider.</td>
</tr>
<tr>
<td>providerAccessToken</td>
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's also mention providerAccessTokenExpiry

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants