Auth API cleanup - First steps #119

ghost · 2016-07-27T16:15:57Z

First step towards #102

This PR provides the following improvements:

Reorganization of the auth API into something more semantically correct
Refactor of the auth.js code to make it simpler and more concise
Extraction of the extractParams functions and checkSignature function in an isofunctional way.

LaurenSpiegel · 2016-07-27T18:27:15Z

lib/auth/auth.js

-auth.check = (request, log, awsService, data) => {
-    log.debug('running auth checks', { method: 'auth' });
+function extractParams(request, log, awsService, data) {
+    log.trace('extracting authentication signature informations');


informations s/b information
do we want to use the "method" log format instead and state the function name?

by log format, do you mean the logged dictionnary ?

as in {method: extractParams}

LaurenSpiegel · 2016-07-29T00:08:51Z

lib/auth/auth.js

+    const payloadChecksum = crypto.createHash('sha256').update(payload)
+                                  .digest('hex');
+    const path = request.path;
+    Object.assign(request, { path: '/' });


This should come out. I did pr to remove but it appears the rebase did not catch it since the function changed.

Could you point me and @AntoninCoulibaly (who will take up these PRs in my absence) to the PR in question ? this would greatly help

alexandre-merle · 2016-08-03T17:11:54Z

lib/auth/auth.js

    const authHeader = request.headers.authorization;
-    // Check whether signature is in header
+    const checkFunctions = {


let's declare this outside the function :)

ghost · 2016-08-05T08:24:15Z

Two little documentations nits for me. Good job otherwise.

koolfunky · 2016-08-05T10:34:33Z

Updated, the commits will be squash and clean after all comments addressed

ghost · 2016-08-05T10:37:25Z

👍 once the lint errors are solved.

koolfunky · 2016-08-05T15:29:23Z

Updated, I removed lib/auth/vault.js and the associated tests

ghost · 2016-08-31T13:26:45Z

@ironman-machine Please validate this step :d (Yes I know, this is kinda new and shiny, and I wanted to play with it first !) 🎱
SCALITY_INTEGRATION_BRANCH=fix/nimrod-build-dep
SCALITY_FEDERATION_BRANCH=ft/dep-patcher-ansible
SCALITY_S3_BRANCH=dev/cleanup/AuthAPI-Step1
SCALITY_VAULT_BRANCH=dev/cleanup/AuthAPI-Step1
SCALITY_VAULTCLIENT_BRANCH=dev/cleanup/AuthAPI

ironman-machine · 2016-08-31T13:51:08Z

Hello @DavidPineauScality,

Starting end to end procedure using the following payload:
{"SCALITY_INTEGRATION_BRANCH":"fix/nimrod-build-dep","SCALITY_FEDERATION_BRANCH":"ft/dep-patcher-ansible","SCALITY_S3_BRANCH":"dev/cleanup/AuthAPI-Step1","SCALITY_VAULT_BRANCH":"dev/cleanup/AuthAPI-Step1","SCALITY_VAULTCLIENT_BRANCH":"dev/cleanup/AuthAPI","DEFAULT_BRANCH":"master","SCALITY_ARSENAL_BRANCH":"dev/cleanup/AuthAPI-Step1"}

Please follow http://ci.ironmann.io/gh/scality/Integration/2827 for circle status.

ghost · 2016-09-08T13:05:01Z

@ironman-machine Please check (rebase still needed)
SCALITY_INTEGRATION_BRANCH=fix/nimrod-build-dep
SCALITY_FEDERATION_BRANCH=ft/dep-patcher-ansible
SCALITY_S3_BRANCH=dev/cleanup/AuthAPI-Step1
SCALITY_VAULT_BRANCH=dev/cleanup/AuthAPI-Step1
SCALITY_VAULTCLIENT_BRANCH=dev/cleanup/AuthAPI

ironman-machine · 2016-09-08T13:05:04Z

Hello @DavidPineauScality,

Starting end to end procedure using the following payload:
{"SCALITY_INTEGRATION_BRANCH":"fix/nimrod-build-dep","SCALITY_FEDERATION_BRANCH":"ft/dep-patcher-ansible","SCALITY_S3_BRANCH":"dev/cleanup/AuthAPI-Step1","SCALITY_VAULT_BRANCH":"dev/cleanup/AuthAPI-Step1","SCALITY_VAULTCLIENT_BRANCH":"dev/cleanup/AuthAPI","DEFAULT_BRANCH":"master","SCALITY_ARSENAL_BRANCH":"dev/cleanup/AuthAPI-Step1"}

Please follow http://ci.ironmann.io/gh/scality/Integration/3046 for circle status.

alexandre-merle · 2016-09-08T15:36:02Z

@ironman-machine You will work, i know that :)
SCALITY_INTEGRATION_BRANCH=fix/nimrod-build-dep
SCALITY_FEDERATION_BRANCH=ft/dep-patcher-ansible
SCALITY_S3_BRANCH=dev/cleanup/AuthAPI-Step1
SCALITY_VAULT_BRANCH=dev/cleanup/AuthAPI-Step1
SCALITY_VAULTCLIENT_BRANCH=dev/cleanup/AuthAPI

ironman-machine · 2016-09-08T15:36:05Z

Hello @alexandre-merle,

Starting end to end procedure using the following payload:
{"SCALITY_INTEGRATION_BRANCH":"fix/nimrod-build-dep","SCALITY_FEDERATION_BRANCH":"ft/dep-patcher-ansible","SCALITY_S3_BRANCH":"dev/cleanup/AuthAPI-Step1","SCALITY_VAULT_BRANCH":"dev/cleanup/AuthAPI-Step1","SCALITY_VAULTCLIENT_BRANCH":"dev/cleanup/AuthAPI","DEFAULT_BRANCH":"master","SCALITY_ARSENAL_BRANCH":"dev/cleanup/AuthAPI-Step1"}

Please follow http://ci.ironmann.io/gh/scality/Integration/3055 for circle status.

alexandre-merle · 2016-09-20T13:34:57Z

@ironman-machine Please, tell me i'm not wrong about rebase :D
SCALITY_INTEGRATION_BRANCH=fix/nimrod-build-dep
SCALITY_FEDERATION_BRANCH=ft/dep-patcher-ansible
SCALITY_S3_BRANCH=dev/cleanup/AuthAPI-Step1
SCALITY_VAULT_BRANCH=dev/cleanup/AuthAPI-Step1
SCALITY_VAULTCLIENT_BRANCH=dev/cleanup/AuthAPI

ironman-machine · 2016-09-20T13:35:00Z

Hello @alexandre-merle,

Starting end to end procedure using the following payload:
{"SCALITY_INTEGRATION_BRANCH":"fix/nimrod-build-dep","SCALITY_FEDERATION_BRANCH":"ft/dep-patcher-ansible","SCALITY_S3_BRANCH":"dev/cleanup/AuthAPI-Step1","SCALITY_VAULT_BRANCH":"dev/cleanup/AuthAPI-Step1","SCALITY_VAULTCLIENT_BRANCH":"dev/cleanup/AuthAPI","DEFAULT_BRANCH":"master","SCALITY_ARSENAL_BRANCH":"dev/cleanup/AuthAPI-Step1"}

Please follow http://ci.ironmann.io/gh/scality/Integration/3799 for circle status.

alexandre-merle · 2016-09-20T16:24:44Z

@ironman-machine let's try again ^^

SCALITY_INTEGRATION_BRANCH=fix/nimrod-build-dep
SCALITY_FEDERATION_BRANCH=ft/dep-patcher-ansible
SCALITY_S3_BRANCH=dev/cleanup/AuthAPI-Step1
SCALITY_VAULT_BRANCH=dev/cleanup/AuthAPI-Step1
SCALITY_VAULTCLIENT_BRANCH=dev/cleanup/AuthAPI

ironman-machine · 2016-09-22T10:23:21Z

Hello @alexandre-merle,

Starting end to end procedure using the following payload:
{"SCALITY_INTEGRATION_BRANCH":"fix/nimrod-build-dep","SCALITY_FEDERATION_BRANCH":"ft/dep-patcher-ansible","SCALITY_S3_BRANCH":"dev/cleanup/AuthAPI-Step1","SCALITY_VAULT_BRANCH":"dev/cleanup/AuthAPI-Step1","SCALITY_VAULTCLIENT_BRANCH":"dev/cleanup/AuthAPI","DEFAULT_BRANCH":"master","SCALITY_ARSENAL_BRANCH":"dev/cleanup/AuthAPI-Step1"}

Please follow http://ci.ironmann.io/gh/scality/Integration/4016 for circle status.

ghost

One question: Was that test removal necessary? I'd rather not see the coverage fall further behind.

Provides the following: - Reduces redundancy by removing known information from module and function names - Simplifies the way the check functions are exported in either auth scheme - Rework the code flow in checkSignature: - Now easier to follow (logic now appears through the code as a two-step operation) - Improved logging (now only provides a few common messages with variable parameters)

- Renames checkSignature into extractParams, as this function mostly extracts the parameters from either the headers or the querystring to identify what kind of auth to use - Renames the data field of the returned data by extractParams into 'params', as those are the real parameters for the signature check function that will soon be extracted from the auth code. - Simplifies the doAuth function as a first step towards its disappearance

- with the refactor of auth API we can now remove unused files * lib/auth/vault.js * tests/unit/auth/v2/errorHandling.js

ghost · 2016-09-26T12:28:07Z

@Michael-Zapata it's mostly due to the fact that my aim is to end with the removal of the doAuth function, so the associated tests are no longer relevant. That being said, other tests are still relying on it, so I'll try restoring the tests.

ghost · 2016-09-26T12:46:20Z

Also, please note that the removed tests were relying on the unused vault.js wrapper that was deleted alongside.

ghost · 2016-09-26T13:03:47Z

Merging on behalf of @rahulreddy since we need to merge the PRs in order (and in sync):

ghost added priority high refactor labels Jul 27, 2016

This was referenced Jul 27, 2016

Use refactored Arsenal's Auth API - Step 1 scality/cloudserver#100

Merged

Update Arsenal's Auth API utilization scality/vaultclient#94

Merged

ghost force-pushed the dev/cleanup/AuthAPI-Step1 branch 2 times, most recently from 3ddc7d9 to c3dfe36 Compare July 27, 2016 18:19

LaurenSpiegel reviewed Jul 27, 2016
View reviewed changes

ghost force-pushed the dev/cleanup/AuthAPI-Step1 branch 2 times, most recently from 9a43094 to 3267fd0 Compare July 27, 2016 19:02

LaurenSpiegel reviewed Jul 29, 2016
View reviewed changes

koolfunky force-pushed the dev/cleanup/AuthAPI-Step1 branch from 3267fd0 to e612e7f Compare August 2, 2016 15:31

alexandre-merle reviewed Aug 3, 2016
View reviewed changes

koolfunky force-pushed the dev/cleanup/AuthAPI-Step1 branch 2 times, most recently from d6ae49d to 2b1c8a9 Compare August 5, 2016 15:26

LaurenSpiegel mentioned this pull request Aug 6, 2016

Pull all required headers to be signed. #134

Merged

ghost force-pushed the dev/cleanup/AuthAPI-Step1 branch from 10077a7 to 776b6a3 Compare August 25, 2016 15:16

alexandre-merle force-pushed the dev/cleanup/AuthAPI-Step1 branch from 776b6a3 to 0727fae Compare September 8, 2016 14:17

alexandre-merle force-pushed the dev/cleanup/AuthAPI-Step1 branch from 0727fae to 8176bd4 Compare September 20, 2016 12:03

alexandre-merle force-pushed the dev/cleanup/AuthAPI-Step1 branch from 7f1c46d to 974bf57 Compare September 22, 2016 09:45

alexandre-merle mentioned this pull request Sep 22, 2016

Dev/cleanup/auth api step1 scality/utapi#51

Merged

ThibaultRiviere approved these changes Sep 22, 2016

View reviewed changes

ghost suggested changes Sep 23, 2016

View reviewed changes

David Pineau and others added 4 commits September 26, 2016 14:17

Re-structure the API into client/server

7da6eca

Clean unused files

87a86ec

- with the refactor of auth API we can now remove unused files * lib/auth/vault.js * tests/unit/auth/v2/errorHandling.js

ghost force-pushed the dev/cleanup/AuthAPI-Step1 branch from 974bf57 to 87a86ec Compare September 26, 2016 12:25

ghost approved these changes Sep 26, 2016

View reviewed changes

ghost merged commit 7dedb0a into master Sep 26, 2016

ghost deleted the dev/cleanup/AuthAPI-Step1 branch September 26, 2016 13:05

alexandre-merle mentioned this pull request Sep 30, 2016

Request Context missing auth info #154

Closed

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Auth API cleanup - First steps #119

Auth API cleanup - First steps #119

ghost commented Jul 27, 2016

LaurenSpiegel Jul 27, 2016

ghost Jul 27, 2016

LaurenSpiegel Jul 27, 2016

LaurenSpiegel Jul 29, 2016

ghost Jul 29, 2016

alexandre-merle Aug 3, 2016

ghost commented Aug 5, 2016

koolfunky commented Aug 5, 2016

ghost commented Aug 5, 2016

koolfunky commented Aug 5, 2016

ghost commented Aug 31, 2016 •

edited by mvaude

Loading

ironman-machine commented Aug 31, 2016

ghost commented Sep 8, 2016

ironman-machine commented Sep 8, 2016

alexandre-merle commented Sep 8, 2016

ironman-machine commented Sep 8, 2016

alexandre-merle commented Sep 20, 2016

ironman-machine commented Sep 20, 2016

alexandre-merle commented Sep 20, 2016 •

edited

Loading

ironman-machine commented Sep 22, 2016

ghost left a comment

ghost commented Sep 26, 2016 •

edited by ghost

Loading

ghost commented Sep 26, 2016

ghost commented Sep 26, 2016 •

edited by ghost

Loading

Auth API cleanup - First steps #119

Auth API cleanup - First steps #119

Conversation

ghost commented Jul 27, 2016

LaurenSpiegel Jul 27, 2016

Choose a reason for hiding this comment

ghost Jul 27, 2016

Choose a reason for hiding this comment

LaurenSpiegel Jul 27, 2016

Choose a reason for hiding this comment

LaurenSpiegel Jul 29, 2016

Choose a reason for hiding this comment

ghost Jul 29, 2016

Choose a reason for hiding this comment

alexandre-merle Aug 3, 2016

Choose a reason for hiding this comment

ghost commented Aug 5, 2016

koolfunky commented Aug 5, 2016

ghost commented Aug 5, 2016

koolfunky commented Aug 5, 2016

ghost commented Aug 31, 2016 • edited by mvaude Loading

ironman-machine commented Aug 31, 2016

ghost commented Sep 8, 2016

ironman-machine commented Sep 8, 2016

alexandre-merle commented Sep 8, 2016

ironman-machine commented Sep 8, 2016

alexandre-merle commented Sep 20, 2016

ironman-machine commented Sep 20, 2016

alexandre-merle commented Sep 20, 2016 • edited Loading

ironman-machine commented Sep 22, 2016

ghost left a comment

Choose a reason for hiding this comment

ghost commented Sep 26, 2016 • edited by ghost Loading

ghost commented Sep 26, 2016

ghost commented Sep 26, 2016 • edited by ghost Loading

ghost commented Aug 31, 2016 •

edited by mvaude

Loading

alexandre-merle commented Sep 20, 2016 •

edited

Loading

ghost commented Sep 26, 2016 •

edited by ghost

Loading

ghost commented Sep 26, 2016 •

edited by ghost

Loading