nerdishbynature / octokit.swift

A Swift API Client for GitHub and GitHub Enterprise
MIT License
488 stars 126 forks source link

Allow for authorization header to be set #154

Closed clayellis closed 1 year ago

clayellis commented 1 year ago

This change allows for consumers of octokit.swift to send requests with authorization other than basic.

The motivation for this change is that Swift scripts that use octokit.swift triggered by actions in a private repo cannot use the default GITHUB_TOKEN with basic authorization, and instead must create and use a secret personal access token. In order to remedy this, the requests need to be authorized with a bearer token using the Authorization authorization header and a Bearer [token] value.

This is the smallest possible change to enable this behavior. In use it would look like:

var tokenConfiguration = TokenConfiguration()
tokenConfiguration.authorizationHeader = "Bearer"
tokenConfiguration.accessToken = token
let octokit = Octokit(tokenConfiguration)
pietbrauer commented 1 year ago

Hey, thanks a lot for the contribution.

I would prefer this to be a separate configuration. Setting the first part of the Authorisation Header freeform feels kind of weird and having a separate configuration would make it more explicit.

In general I wonder where this change in behaviour stems from as the TokenConfiguration was meant for using it with personal access tokens.

clayellis commented 1 year ago

Happy to contribute!

The GitHub API requires bearer authorization in some cases (for instance, if the organization enforces SAML SSO): https://docs.github.com/en/rest/overview/other-authentication-methods#authenticating-for-saml-sso

Given that Octokit currently requires a TokenConfiguration (even OAuthConfiguration eventually boils down to a TokenConfiguration), how would you propose going about this?

Perhaps a simpler change would be to introduce an alternate, explicit initializer on TokenConfiguration. The first one continues to use basic authorization with no breaking changes. The second is explicitly a bearer authorization token.

public struct TokenConfiguration {
    ...

    public init(_ token: String? = nil, url: String = githubBaseURL, previewHeaders: [PreviewHeader] = []) {
        apiEndpoint = url
        accessToken = token?.data(using: .utf8)!.base64EncodedString()
        previewCustomHeaders = previewHeaders.map { $0.header }
    }

    public init(bearerToken: String, url: String = githubBaseURL, previewHeaders: [PreviewHeader] = []) {
        apiEndpoint = url
        authorizationHeader = "Bearer"
        accessToken = bearerToken
        previewCustomHeaders = previewHeaders.map { $0.header }
    }
}

Then in use this looks like:

let octokit = Octokit(TokenConfiguration(bearerToken: token))
pietbrauer commented 1 year ago

Yes, even better. I'm happy with the second initialiser.

Could you update your PR?

clayellis commented 1 year ago

Updated 👍

clayellis commented 1 year ago

Updated to use the alternative. Should be ready to go now! Thanks for the timely feedback @pietbrauer