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.

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

feat(providers): add Notion provider #2932

Closed
wants to merge 3 commits into from
Closed

feat(providers): add Notion provider #2932

wants to merge 3 commits into from

Conversation

balazsorban44
Copy link
Member

@balazsorban44 balazsorban44 commented Oct 10, 2021

Currently trying to figure out why the owner is not correctly returned.

https://developers.notion.com/docs/authorization#exchanging-the-grant-for-an-access-token

Fixes #2905

Proof after login:

image

The image does not show because I got rate-limited with all my failed attempts. 😅

@github-actions github-actions bot added core Refers to `@auth/core` providers labels Oct 10, 2021
@balazsorban44 balazsorban44 temporarily deployed to Preview October 10, 2021 21:29 Inactive
@codecov-commenter
Copy link

codecov-commenter commented Oct 10, 2021

Codecov Report

Merging #2932 (ab57abb) into beta (58a98b6) will decrease coverage by 0.22%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             beta    #2932      +/-   ##
==========================================
- Coverage   14.07%   13.85%   -0.23%     
==========================================
  Files          85       86       +1     
  Lines        1357     1379      +22     
  Branches      392      399       +7     
==========================================
  Hits          191      191              
- Misses       1107     1128      +21     
- Partials       59       60       +1     
Impacted Files Coverage Δ
src/providers/notion.ts 0.00% <0.00%> (ø)
src/server/lib/oauth/authorization-url.js 0.00% <0.00%> (ø)
src/server/lib/oauth/client.ts 0.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 58a98b6...ab57abb. Read the comment docs.

@github-actions
Copy link

github-actions bot commented Oct 10, 2021

🎉 Experimental release published on npm!

npm i next-auth@0.0.0-pr.2932.3e8c32f4
yarn add next-auth@0.0.0-pr.2932.3e8c32f4

@justjake
Copy link

justjake commented Oct 11, 2021

Hello, thanks for taking an interest in the Notion OAuth APIs. I appreciate you taking the time to add first-party support to next-auth. I haven't tried next-auth@4 yet, but I'm attaching the complete source of my next-auth@3 provider below.

I think the difficulties you're encountering are related to a missing Notion-Version HTTP header. This header is used to decide the version of the Notion API at request time to the server. Without such a header, you'll receive response types and behaviors that are several months out of date.

This is copy-pasted out of the source of a toy app I work on sometimes in my personal time, so I appoligize for any confusing code or poor code quality.

import { GetUserResponse } from "@notionhq/client/build/src/api-endpoints";
import { DefaultProfile } from "next-auth";
import { OAuthConfig } from "next-auth/providers";

export interface NotionProfile extends DefaultProfile {
  /**
   * The bot_id, which is an ID unique to a single workspace, 
   * not the human's user id. Using bot_id maps each authorization
   * to a different Account in next-auth@3.
   */
  id: string;
  /** The Notion human's email.  */
  email: string;
  /** The Notion human's user id */
  user_id: string;
  /** The ID of the workspace this user authenticated with */
  workspace_id: string;
  workspace_icon?: string;
  workspace_name?: string;
}

export type NotionProvider = OAuthConfig<
  // idk what i'm doing
  NotionProfile & Record<string, unknown>
>;

interface NotionProviderConfig extends Partial<NotionProvider> {
  clientId: string;
  clientSecret: string;
  notionVersion?: string;
  baseUrl?: string;
}

const DEFAULT_BASE_URL = "https://api.notion.com";

export function NotionProvider(options: NotionProviderConfig): NotionProvider {
  const {
    clientId,
    clientSecret,
    notionVersion: givenNotionVersion,
    baseUrl: givenBaseUrl,
    headers,
    ...overrides
  } = options;
  // Note: the Notion API is versioned using the Notion-Version HTTP header.
  // More info: https://developers.notion.com/reference/versioning
  const notionVersion = givenNotionVersion || "2021-08-16";
  const baseUrl = givenBaseUrl || DEFAULT_BASE_URL;
  const bearer = base64(`${clientId}:${clientSecret}`);

  return {
    id: "notion",
    name: "Notion",
    type: "oauth",
    version: "2.0",
    params: {
      grant_type: "authorization_code",
    },
    accessTokenUrl: `${baseUrl}/v1/oauth/token`,
    authorizationUrl: `${baseUrl}/v1/oauth/authorize?response_type=code&owner=user`,
    clientId,
    clientSecret,
    headers: {
      "Notion-Version": notionVersion,
      Authorization: `Basic ${bearer}`,
      ...headers,
    },
    scope: "",

    // Fetch user information, and produce an "account" per workspace.
    // TODO: a seperate fetch isn't needed since the OAuth token response
    // returns an `owner` field with this data, but next-auth@3 seems to require one.
    profileUrl: `${baseUrl}/v1/users/me`,
    async profile(fetchedUser, tokens) {
      const token: NotionOAuthToken = tokens as any;
      const botUser: GetUserResponse = fetchedUser as any;
      const personUser = getPersonUser(botUser);
      const result: NotionProfile = {
        email: personUser.person.email,
        id: botUser.id,
        user_id: personUser.id,
        name: personUser.name || undefined,
        image: personUser.avatar_url || undefined,
        workspace_icon: token.workspace_icon,
        workspace_id: token.workspace_id,
        workspace_name: token.workspace_name,
      };
      return result as NotionProfile & Record<string, unknown>;
    },

    ...overrides,
  };
}

export interface NotionOAuthToken {
  access_token: string;
  token_type: "bearer";
  bot_id: string;
  workspace_id: string;
  workspace_name?: string;
  workspace_icon?: string;
  owner: { type: "workspace", workspace: true } | { type: "user", user: PersonUser };
}

function base64(string: string): string {
  return Buffer.from(string).toString("base64");
}

type PersonUser = Extract<GetUserResponse, { type: "person" }>;

function getPersonUser(user: GetUserResponse): PersonUser {
  if (user.type === "person") {
    return user;
  }

  if (user.type === "bot") {
    const bot = user.bot;

    if (bot.owner.type === "workspace") {
      throw new Error("Workspace bots don't have a person");
    }

    if (!("type" in bot.owner.user)) {
      throw new Error("Owner user object has no type field");
    }

    return getPersonUser(bot.owner.user);
  }

  throw new Error(`Notion user object has unknown type: ${(user as any).type}`);
}

@balazsorban44
Copy link
Member Author

balazsorban44 commented Oct 11, 2021

Thank you @justjake! Unfortunately adding a Notion-Version: 2021-08-16 header to the token request had no effect. Whatever I do, owner is always returned as { type: 'workspace', workspace: true }. I created a new workspace from scratch, with a new e-mail and a new integration, where the first thing I did was to change to OAuth. 😕

By the way, is it possible to set Notion-Version as part of the URL, a query parameter, or a body? The header is the only part of a request that is cumbersome to set even with openid-client (the underlying OAuth library) 😅

@balazsorban44
Copy link
Member Author

balazsorban44 commented Oct 11, 2021

I have now asked two individuals how I know are using Notion, one of them did not use this API, the other one got the user object in owner, with an identical request. NOTE, they did not even have to set a Notion-Version header.

The issue might be in my Notion configuration somehow.

I created an integration and set it to OAuth like so:

image

Here is my consent screen:
image

What am I missing?

@justjake
Copy link

Maybe what’s happening here is that when you create an integration, it gets an initial token as a workspace token in its “home” workspace. When you make repeated OAuth requests for the same (user, workspace) pair you’ll receive the same token response until the integration is de-authorized on the Notion side of things.

We could test this theory two ways:

I also have more example code here which is working fine: https://github.com/justjake/passport-notion/blob/main/lib/passport-notion/strategy.ts

In any case, I apologize for the confusing API experience. I’ll pass this thread along to the API & Platform team as feedback. Thank you for your time.

@justjake
Copy link

By the way, is it possible to set Notion-Version as part of the URL, a query parameter, or a body? The header is the only part of a request that is cumbersome to set even with openid-client (the underlying OAuth library) 😅

I don’t think so. headers is a Provider option in v3 and working fine, is this removed in v4?

@balazsorban44
Copy link
Member Author

balazsorban44 commented Oct 11, 2021

The top-level header option is removed because it did not make sense there. I might add it back later on in another form. 99% of the providers don't need it (read: only notion would require it so far out of the 30+ providers I've checked, but I have a feeling it's not my problem here)

I'll check your suggestions, but both the workspace/integration/account has been created from scratch, just an FYI for this purpose, so everything started off "clean".

I'll report back with my findings thanks for the help!

UPDATE: OK, I did something in Notion that made it work. 😅 I'll continue and push an update to this PR soon.

@balazsorban44 balazsorban44 temporarily deployed to Preview October 11, 2021 13:23 Inactive
@balazsorban44
Copy link
Member Author

balazsorban44 commented Oct 11, 2021

Awesome, I think I got it working! Thank you for the help! Here is what I get when logging in:

image

The image does not show because I got rate-limited with all my failed attempts. 😅

here is the profile object:

{
  workspace_id: '0000-0000-0000-0000',
  workspace_name: "Balázs Orbán's Notion",
  workspace_icon: 'some.url',
  bot_id: '0000-0000-0000-0000',
  type: 'person',
  user: null,
  object: 'user',
  id: '0000-0000-0000-0000',
  name: 'Balázs Orbán',
  avatar_url: 'some.url',
  person: { email: 'info@balazsorban.com' },
  email: 'info@balazsorban.com'
}

@justjake would this be sufficient?

@balazsorban44 balazsorban44 marked this pull request as ready for review October 11, 2021 13:25
@justjake
Copy link

justjake commented Oct 11, 2021

Nice work. Before you merge please make sure that the provider produces a unique Account per workspace you auth with - but links to the same User. Because integrations are granted permission on a per workspace basis in Notion, an iteration needs a separate access_token per workspace.

@balazsorban44
Copy link
Member Author

balazsorban44 commented Oct 11, 2021

I think the workspace part is up to the user. In most default cases, I believe we want to keep it simple as the other providers.

You can certainly override the profile callback and define your own id however you want, but in my head, a user ID should be the user's id. In next-auth, you usually have a single user who connects to multiple accounts, but here we are discussing if a single account can have multiple workspaces (I think?).

Here is an overview of the next-auth models: https://next-auth.js.org/adapters/models

In next-auth you have a User (not a Notion user) and accounts (eg.: a Notion user) connected to them. Here, it sounds like that Notion user (account) should be able to have multiple accounts (workspaces) themselves.

Am I understanding it correctly?

@balazsorban44 balazsorban44 requested a review from ndom91 October 11, 2021 13:48
@justjake
Copy link

Since OAuth tokens are issued per-workspace, I think it's better to treat a Notion OAuth token as an Account, rather than treating the Notion User as an Account.

In Notion, it's quite common to be in multiple workspaces. My workspace switcher for my personal Notion account has 6 workspaces.

screenshot

image

If the provider uses the Notion User's id for the Account ID (one Account per Notion User), I won't be able to connect my next-auth app to more than one workspace at a time -- I'd have to re-authenticate each time I want to act in a different workspace. That would be quite frustrating, especially for power users or consultants who work in many more workspaces.

Instead, it's better to use the OAuth token's bot_id field as the provider Account ID. That way, next-auth apps that use the Notion provider can easily implement multiple workspace support by querying for Accounts from the Notion provider, and it's easy to add a relational linkage between a NextAuth Account and the related resources fetched from the Notion API.

For example in the app I'm working on, i have my own kind of workspace switcher to select Notion workspaces, and if I add support for Github or otherwise those accounts will show up in the list easily.

image

@balazsorban44
Copy link
Member Author

I see, thanks for the the context, very helpful!

I don't think we have an OOB solution for workspaces/teams at the moment, because it's a concept on another level I think, but it might be easier to solve than I think, I'm just a bit busy these days. I'll try to find some time and discuss it in the coming days and see if we have a neat solution for this that don't require much work from the user.

We do have events and callbacks that might be useful, but I'll see if we can do it without them first.

https://next-auth.js.org/configuration/callbacks
https://next-auth.js.org/configuration/events

Thank you for keeping interest and helping out with this! 💚

@jordn
Copy link

jordn commented Oct 12, 2021

Thanks for adding this integration. Literally just been trying to do my own oauth2 notion integration.

Just pointing out (sorry if i'm skipping context here) that Notion's integrations are now user-level (and all old ones switch over on 19th October) https://developers.notion.com/docs/authorization

@balazsorban44
Copy link
Member Author

balazsorban44 commented Oct 12, 2021

Thank you. Integrations might be, but as pointed out above, workspaces still get their own access tokens, so we cannot handle Notion users as in other providers.

I hope to find a solution that does not need Notion specific changes to the core, as I used a great deal of my time to remove any remaining provider-specific handling in v4.

With v4, the user has full and granular (optionally) control of the OAuth flow, so I hope we can use that here somehow. I'll have another look later on, but I'm happy for any additional information/comments.

@justjake
Copy link

justjake commented Oct 12, 2021

Is there a technical reason why mapping a Notion workspace-specific OAuth token to an Account doesn’t work in next-auth@4? This approach works for me in next-auth@3, and the Accounts are linked to the correct next-auth User based on the Profile information.

This is not so different from e.g. Slack, where there is an Account per workspace, and multiple accounts can have the same email; the difference is Slack doesn’t explicitly expose the underlying user via the API.

@balazsorban44
Copy link
Member Author

balazsorban44 commented Oct 12, 2021

This is not so different from e.g. Slack, where there is an Account per workspace, and multiple accounts can have the same email; the difference is Slack doesn’t explicitly expose the underlying user via the API.

It never came up. Usually Slack integrations target a single workspace, maybe? 🤷‍♂️ So the workspace information/id can be like an environment variable instead of being dynamic like here. This is just my guess BTW, but I haven't seen anything like that in related issues, not with Slack, or any other providers we have. So it's a first usecase. This doesn't make it invalid, just hard to wrap my head around to come up with the right solution.

This approach works for me in next-auth@3

If it worked in v3 it should in v4, as we didn't explicitly remove functionality, if anything at all, we significantly simplified the public APIs (#2361, nextauthjs/adapters#171), I just feel like this extra layer of complexity will confuse users (as it confuses me as a first-time user of Notion).

As @ndom91 pointed out to me, this is only a real problem for when you want to use an Adapter. When the session/user is stateless (persisted in a JWT formatted session cookie), you kind of have to re-authenticate between different workspaces anyway, as you would when signing in with eg. GitHub instead of Google.

Someone told me, if Notion wanted to follow other providers, we would call its button "Sign-in with Notion workspace"

@jordn
Copy link

jordn commented Oct 12, 2021

Hi. Trying to follow along here but thought i'd just add to the conversation. I don't think I have opinions about the IDs/workspaceID.

I had the issue marked above (using the v3 next-auth and @justjake's example)... first time through owner was just a bot (i think) owner: { type: 'workspace', workspace: true }. I deactivated the integration on notion and tried again and i did get the user ID etc.

So... that's all a bit weird. Not sure I understand why that was the case or whether it will always be the case. (good chance i'm doing something wrong here too).

And other oddity is that on re-login with the same workspace I select the pages given access each time. I wouldn't really expect that.

@feliche93
Copy link

@jordn or @balazsorban44 were you able to create a working provider integration? I tried doing it myself and get an [next-auth][error][GET_AUTHORIZATION_URL_ERROR] although the authorization url perfectly works. Could I take your example from the PR or could you post any working version? Thanks 🙏

@balazsorban44
Copy link
Member Author

This seemed to be a bit more complicated than expected because of Notion workspaces.

There is a good amount of useful code in this PR, but the confusion around the different token owners would probably be confusing for users of other providers, that do not have those concepts. We might revisit this after v4 has been released to v4, or we could add a "simplified" version, declaring the constraints in the documentation.

@balazsorban44 balazsorban44 deleted the feat/notion branch December 3, 2022 11:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Refers to `@auth/core` providers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants