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

feat: Support custom auth schemes for Github API #1086

Conversation

soceanainn
Copy link
Contributor

Background

When looking to support custom authentication with Github (by proxying to Github API through a custom authenticated API), the token auth-scheme in the Authorization header may not be appropriate.

For example, when looking to use AWS Lambda function to proxy Github API requests and using AWS Cognito for authentication (Cognito is the user management service offered by AWS out of the box), authenticated requests to the Lambda function must use Bearer scheme.

Note: Github API does seem to accept either Bearer or token, and even appears to prefer and sometimes mandate Bearer according to their API docs:
https://docs.github.com/en/rest/authentication/authenticating-to-the-rest-api?apiVersion=2022-11-28#about-authentication

However - changing this would be a potentially breaking change and I'm hesitant to change something fundamental to the Github backend implementation in this PR. This PR would allow us to test that the normal Github implementation works with the Bearer scheme and change that to the new default in future (if desired) - but that's well outside the scope of this change.

Changes

  1. Allow configuring the authScheme used in Authorization headers for Github backend, defaulting to the expected default value of token.
  2. Add basic test ensuring that overriding authScheme works for API spec. No equivalent tests exist in implementation spec so I haven't added equivalent tests for that.

Copy link

netlify bot commented Feb 19, 2024

Deploy Preview for staticjscms ready!

Name Link
🔨 Latest commit d0d4d2e
🔍 Latest deploy log https://app.netlify.com/sites/staticjscms/deploys/65d37bcb97fac8000889e98f
😎 Deploy Preview https://deploy-preview-1086.staticcms.org
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Feb 19, 2024

Deploy Preview for demo-staticjscms ready!

Name Link
🔨 Latest commit d0d4d2e
🔍 Latest deploy log https://app.netlify.com/sites/demo-staticjscms/deploys/65d37bcb6bbf1f00088c6192
😎 Deploy Preview https://deploy-preview-1086.demo.staticcms.org
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@soceanainn soceanainn changed the title Support custom auth schemes for Github API (needed for proxies) feat: Support custom auth schemes for Github API Feb 19, 2024
packages/core/src/backends/github/__tests__/API.spec.ts Dismissed Show dismissed Hide dismissed
packages/core/src/backends/github/__tests__/API.spec.ts Dismissed Show dismissed Hide dismissed
packages/core/src/backends/github/API.ts Dismissed Show dismissed Hide dismissed
Copy link

codecov bot commented Mar 5, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 55.83%. Comparing base (9379d3c) to head (d0d4d2e).
Report is 5 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1086   +/-   ##
=======================================
  Coverage   55.82%   55.83%           
=======================================
  Files         259      259           
  Lines       12360    12362    +2     
  Branches     3108     3110    +2     
=======================================
+ Hits         6900     6902    +2     
  Misses       5048     5048           
  Partials      412      412           

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

@KaneFreeman KaneFreeman merged commit 17b32f0 into StaticJsCMS:main Mar 5, 2024
19 of 20 checks passed
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.

2 participants