Adding OAuth revoke and introspect endpoints

[vincedbowen]

As described in Issue #260,

OAuth revoke and introspect endpoints got added to the JS SDK: makenotion/notion-sdk-js#552

This PR adds that functionality to this SDK.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (2aac1da) to head (d7c985f).
Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff            @@
##              main      #261   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            7         7           
  Lines          304       323   +19     
=========================================
+ Hits           304       323   +19     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[ramnes]

LGTM overall, just need some clarification on the Authorization header.

[vincedbowen]

I think some documentation updates would be great, but I will wait for any feedback on codebase changes before doing that so I don't waste anytime documenting unwanted changes :-)! The tests are a bit annoying since the developer effectively has to burn two tokens, but that was the only strategy I found to work. Let me know if any changes should be made!!

Also if you have any general best practices for this codebase, or PRs that I am not following, please let me know 😄

[ramnes]

This is weird, how does the JS SDK handle this?

[ramnes]

See comments.

[ramnes]

Tests are failing. :)

Why do you say two tokens have to be burnt? I only see one revoke here.

[vincedbowen]

Tests are failing. :)

Why do you say two tokens have to be burnt? I only see one revoke here.

One token has to be intercepted to be used in the generate token endpoint, and then another in the introspect and revoke. So I guess only the revoke token has to be burned, but the developer has to create a code and a token. I was thinking one token could be used, but then the generate token would have to be run, and the token would have to be set on the fly, before the introspect and revoke test could be run, which I am not sure is desirable behavior. Let me know!

Looks like this may be the reason the tests are failing in Python 3.9 and lower. Could this possibly be the reason? If so, do you have any suggestions? If not, let me know and I can investigate further.

[ramnes]

Let's use a TypedDict :P

(and drop support for Python 3.7 since it has been implemented in 3.8)

[vincedbowen]

Let me know what you think of the current changes since your last comments. If those look better, I will fix the rest of the pending changes :-).

[ramnes]

Let me know what you think of the current changes since your last comments. If those look better, I will fix the rest of the pending changes :-).

Yeah it's going in the right direction, keep up the good work! You've already done more than I thought necessary when I opened #261. Sorry if the PR feels a bit tedious, this library is vastly used in the wild so the last thing I want is more work fixing things down the line. :)

[vincedbowen]

Let me know what you think of the current changes since your last comments. If those look better, I will fix the rest of the pending changes :-).

Yeah it's going in the right direction, keep up the good work! You've already done more than I thought necessary when I opened #261. Sorry if the PR feels a bit tedious, this library is vastly used in the wild so the last thing I want is more work fixing things down the line. :)

No worries, apologies if it has taken a number of iterations to produce high quality code!

I think I have resolved all of your reviews, besides the error handling in the case of the 401 response. To Clarify why I am confused on this:

If I do nothing, an APIResponseError is not raised, and the error is instead raised as an HTTPResponseError. This is because the current code checks for the string code, and matches that to a number status. An example of the response body where this works just fine is here (if you click the 403 example response). The authentication APIs do not respond with the code in the response body, and thus the error cannot be matched. The JS SDK only tests for a successful response as seen here, so this error never needs to be handled. I see three ways to solve this issue:

1. Let the error be passed to the HTTPResponseError (This seems a bit hacky, as this is an APIResponseError where Notion is telling the user they are unauthorized)
2. Use the actual response code from httpx instead of accessing the code through the response body
3. Raise an issue or conversation with the folks who run the JS SDK about why the response errors are handled in this way

Let me know what you think of those options, or if you have an entirely different idea on how to solve that :-).

Additionally here are the steps I have to take in order to create new cassettes for the tests. This will explain why I have to generate two token. I don't think there is a way to avoid this because the token would have to be generated on the fly and the environment variable would have to be set from within the tests. I don't think that is a solid solution because the tests shouldn't have to be executed in a definitive order like that.

1. I created a public integration
2. I set the environment variables NOTION_CLIENT_ID, NOTION_CLIENT_SECRET, and NOTION_REDIRECT_URI
3. I followed the authorization URL in my public integration (https://api.notion.com/v1/oauth/authorize?client_id=<YOUR_CLIENT_ID>&response_type=code&owner=user&redirect_uri=<YOUR_REDIRECT_URI>)
   i. I selected the pages and allowed access for my account
   ii. I grabbed the code from the redirect URI http://localhost:3000/callback?code=<YOUR_CODE>&state=
4. I set this as the NOTION_CODE. This is the code that will be used to actually generate a token in the test_token(client, redirect_uri, code, client_id, client_secret) test.
5. Because of the aforementioned reasons I followed step 3 again to get a new code
6. I used a cURL command to generate a token from this code. This token is what will be used for the introspect and revoke tests.

curl --location 'https://api.notion.com/v1/oauth/token' \
--header 'Content-Type: application/json' \
--header 'Notion-Version: 2022-06-28' \
--header 'Authorization: Basic "<YOUR_Base64Encoded($client_id:$client_secret)>"' \
--data '{
  "grant_type": "authorization_code",
  "code": "<YOUR_CODE>",
  "redirect_uri": "<YOUR_REDIRECT_URI>"
}'

7. I grabbed the "access_token" from the response body and set that as the NOTION_TOKEN environment variable

I understand this is a bit of a clunky and hacky solution, but it is the best I could think of right now. If you have a better way to go about this, please let me know! Otherwise I will update the developer docs to include these steps.

Apologies for the wordy comment 😅