Skip to content
This repository has been archived by the owner on May 3, 2022. It is now read-only.

Should be able to authenticate using a URL query parameter #19

Open
singingwolfboy opened this issue Apr 10, 2020 · 5 comments
Open

Should be able to authenticate using a URL query parameter #19

singingwolfboy opened this issue Apr 10, 2020 · 5 comments

Comments

@singingwolfboy
Copy link

Some APIs require authentication using a query parameter in the URL. For example, the Clubhouse API has examples that look like this:

curl -X GET \
  -H "Content-Type: application/json" \
  -L "https://api.clubhouse.io/api/v3/member?token=$CLUBHOUSE_API_TOKEN"

To resolve this issue, auth.ts should define and export a class that can handle automatically attaching a token to the query parameters in the URL. For example:

export class QueryParamHandler implements ifm.IRequestHandler {
    param: string
    token: string;

    constructor(token: string, param="token") {
        this.token = token;
        this.param = param;
    }

    prepareRequest(options:any): void {
        // not sure where to find request URL, so this is pseudocode...
        if (this.url.contains("?")) {
            this.url += `&${this.param}=${this.token}`;
        } else {
            this.url += `?${this.param}=${this.token}`;
        }
    }
}
@bryanmacfarlane
Copy link
Member

bryanmacfarlane commented Apr 10, 2020

Thanks for creating an issue and the suggestion!

Just wanted to point out that the curl example above is possible with the http-client. Get takes an url which can include query string parameters

https://github.com/actions/http-client/blob/master/index.ts#L165

If used in an action (purpose of lib), the action can have an input and the consumers of it can create a secret in the repo settings, provide the value in the workflow file with ${{ ... }} giving the action access to it. The action can format it into the url.

So I'm very hesitant to add the complexity of a provider layer url.

Hope that helps.

@bryanmacfarlane
Copy link
Member

@ericsciple

@singingwolfboy
Copy link
Author

Of course, it's entirely possible for the developer to add the authentication token to the query string of the HTTP request manually. It's also entirely possible for the developer to add the authentication token to the headers, which is exactly what the existing classes in auth.ts are designed to do.

The goal in this case is developer experience. It's much more error-prone for the developer to have to remember to pass the token param every time they make an HTTP request. The existing auth.ts classes allow the developer to set the authentication information once and then not worry about it anymore. That should be possible regardless of whether the authentication information is in the headers, in the query string, in the HTTP POST body, or anywhere else.

This authentication system is clearly designed to be extensible, to accommodate this ideal of setting authentication in one place regardless of how it is applied to the HTTP request. However, I'm trying to build an Action based on @actions/typescript-action, which enables strict mode in Typescript. Because this project is not compatible with strict mode in Typescript (#22), and specifically the authentication classes are defined to be incompatible with this mode (#21), I can't simply build this authentication class in my project: it won't type-check.

@ericsciple
Copy link
Collaborator

Does the specific API in question allow auth header instead? Auth header is better to avoid being captured in logs (common for url to be captured in server logs).

Request functions accept dictionary today. Easier than manipulating URL each request. Could simplify slightly in the future, if the constructor accepted a dictionary of headers to apply to each request.

@bryanmacfarlane
Copy link
Member

I disagree that it's the design goal to solve query string params.

The design goal of the auth handlers was to be able to delay inject headers for all calls downstream. For something as explicit as creating a get request on an url where the data is part of the url, simply formatting it into the url is clearer, simpler and more straight forward without adding yet another provider layer.

Strict type checking is an orthogonal issue.

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

No branches or pull requests

3 participants