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

HMAC authenticaion method #2

Merged
merged 2 commits into from
Nov 13, 2017
Merged

HMAC authenticaion method #2

merged 2 commits into from
Nov 13, 2017

Conversation

OndraM
Copy link
Member

@OndraM OndraM commented Nov 9, 2017

Only a basic building block of client internals.

Will be used as custom authentication method of HTTPlug Authentication plugin.

Something like

new PluginClient(
    HttpClientDiscovery::find(),
    [
        new AuthenticationPlugin(new HmacAuthentication($this->apiKey)),
    ]
);

(This will be part of Matej\Client internals, so end-clients will not have to set it up.)

return $this->createRequestWithParams($request, $params);
}

private function addAuthenticationParams(RequestInterface $request): array
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe getAuthenticationParams or createAuthenticationParams would be better names.

Copy link
Member Author

@OndraM OndraM Nov 12, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've been thinking about this one before and also considered both of those suggestions. But I didn't like them as well - the params are actually being added to params already present in Request. So they are not being created, nor just "get" (get = no side effects). But I don't like addAuthenticationParams either.

So what about addAuthenticationToParamsFromRequest($request)?

@OndraM OndraM merged commit 59a3070 into master Nov 13, 2017
@OndraM OndraM deleted the feature/authentication branch November 13, 2017 12:51
@OndraM OndraM added this to the 0.9 milestone Nov 23, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants