Feature: Add Auth middleware and Authenticator. #26

kokokuo · 2022-08-02T06:51:06Z

Description

Support to provide built-in basic authenticator and authenticator abstract class for user customizing.

The Authenticator provides the authentication feature to make our user ( DA/DE ) could define the access way (e.g: username/password or token or credential ) and read each DC user's role with some customized attribute ( e.g: department ) for authorization used.

For the implementation, Vulcan provides three built-in authenticators and sets each authenticator option under options of auth.

BasicAuthenticator

The HTTP Basic authenticator uses the authorization header Basic <credential> (insensitive for auth type) to authenticate. The credential encode base64 from <username>:<password>, and it supports two set user credentials method defined in project YAML.

auth: 
  options: 
    // identifier for basic auth
    basic:
      htpasswd-file:
         path: <file-path>
         users:
           - name: user1
              attr:
                role: 'engineer' 
 
      users-list: 
        - name: user3
           md5Password: <md5-password>
           attr: 
             role: 'engineer'

The above htpasswd-file could set the user and md5 hash password in the file, user-list could let you set in yaml directly.

After you authenticate failed by request with authorization header Basic <credential>, then it will return the result with WWW-Authenticate header in the response.

SimpleTokenAuthenticator

The Simple Token authenticator uses the authorization header Simple-Token <credential> to authenticate. The credential could be of any value and it authenticates from your user credentials in project YAML.

auth: 
  options: 
     // identifier for basic auth
    simple-token:
     - name: user3
        token: <any-value>
         attr: 
           role: 'engineer'

The above token in config is also could be any value, so the simple token authenticator is just a simple way to authenticate the same credentials from the authorization header Simple-Token <credential> (insensitive for auth type). In SimpleTokenAuthenticator, it will respond a JSON failed result like below when you authenticate failed:

{
   "type": "simple-token",
  "message": "<the error message>"
}

PasswordFileAuthenticator

The Password File authenticator uses the authorization header Password-File <credential> (insensitive for auth type) to authenticate. The credential encodes base64 from <username>:<password>, and it authenticates by the password file, we could set the path in project YAML.

auth: 
  options: 
    // identifier for password-file auth
    password-file:
       path: <file-path>
       users:
         - name: user1
            attr:
               role: 'engineer'

The password file should be <username>:<bcrypt-password> format in each line, it means the password need to encode bcrypt with round 10 first and then add the file. If not, we will show an error message to the notice user.

If authenticate failed, it will respond json format like the below:

{
   "type": "password-file",
  "message": "<the error message>"
}

How to use it

You can use our built-in authenticator and define the property with its identifier under options of auth module name in project config YAML to get the auth default defined information. You could define one to multiple authenticator options in YAML depend on what authentication you want.

Please make sure to keep the same attr for consistency when you define the same user data in each different authenticator, but you could also omit this consistency when you want.

auth: 
  options: 
     basic: 
       ...
     simple-token: 
       ...
     password-file: 
       ...

Sample: doing the auth by the basic authenticator

Step1: Add a user eason by using basic authentication with a checked token:

auth: 
     options:
      basic:
        users-list: 
          - name: eason
             md5Password: '098f6bcd4621d373cade4e832627b4f6'
          attr:                 
             role: admin

Step2: Send an API request with the user's basic auth token and passed

Authorization: Basic ZWFzb246MDk4ZjZiY2Q0NjIxZDM3M2NhZGU0ZTgzMjYyN2I0ZjY=

Step3: Passed, do the authorization in SQL file

{% if context.user.attr.role != 'admin' %}
	error "No permission to do the query."
{% endif %}

select * from orders

⚠️ Notice

The new version authenticator which connected which client-server is still designing, so I will create a new branch and pr to develop it after the new design confirms in the next meeting.

How To Test / Expected Results

For the test result, please see the below test cases that passed the unit test:

Commit Message

caac5ee feat(serve): add auth middleware and authenticator
51444cb feat(serve): add authenticator and auth middleware test cases
898f0db feat(serve): add cursor, offset, keyset pgination strategy test cases.
ba1b91f fix(serve): make authenticator and auth middleware have different result status.
8e9c780 fix(serve): refactor http basic and add simple-token, password-file authenticator.
a1c8572 refactor(serve): add http basic authenticator
c95709b fix(serve): update authenticator and auth middleware test cases

kokokuo · 2022-08-17T06:20:03Z

The rebase has done from #23 which based on #25

codecov-commenter · 2022-08-19T01:27:49Z

Codecov Report

Base: 91.36% // Head: 91.39% // Increases project coverage by +0.02% 🎉

Coverage data is based on head (dde285f) compared to base (dc2cf86).
Patch coverage: 89.07% of modified lines in pull request are covered.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop      #26      +/-   ##
===========================================
+ Coverage    91.36%   91.39%   +0.02%     
===========================================
  Files          199      205       +6     
  Lines         2630     2871     +241     
  Branches       280      336      +56     
===========================================
+ Hits          2403     2624     +221     
- Misses         177      184       +7     
- Partials        50       63      +13

Impacted Files	Coverage Δ
packages/serve/src/containers/types.ts	`100.00% <ø> (ø)`
...erve/src/lib/middleware/response-format/helpers.ts	`100.00% <ø> (ø)`
...src/lib/pagination/strategy/cursorBasedStrategy.ts	`100.00% <ø> (+20.00%)`	⬆️
...src/lib/pagination/strategy/offsetBasedStrategy.ts	`100.00% <ø> (+20.00%)`	⬆️
...ages/serve/src/lib/pagination/strategy/strategy.ts	`100.00% <ø> (ø)`
...s/serve/src/lib/response-formatter/csvFormatter.ts	`81.57% <ø> (ø)`
.../serve/src/lib/response-formatter/jsonFormatter.ts	`79.31% <ø> (ø)`
...s/serve/src/models/extensions/responseFormatter.ts	`100.00% <ø> (ø)`
...ges/serve/src/models/extensions/routeMiddleware.ts	`64.70% <ø> (-10.30%)`	⬇️
...erve/src/lib/route/route-component/graphQLRoute.ts	`50.00% <25.00%> (-5.56%)`	⬇️
... and 38 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

oscar60310

I have some suggestions about auth design. Thanks for adding test and factorying!

oscar60310 · 2022-08-18T07:45:24Z

packages/serve/src/models/extensions/authenticator.ts

+  ): Promise<AuthResult>;
+}
+
+@VulcanExtension(TYPES.Extension_Authenticator)


Suggested change

@VulcanExtension(TYPES.Extension_Authenticator)

@VulcanExtension(TYPES.Extension_Authenticator, { enforcedId: true })

Thanks for suggesting, I have fixed the part to add it.

oscar60310 · 2022-08-19T02:53:12Z

packages/serve/src/lib/auth/basicAuthenticator.ts

+    ansToken = matched
+      ? (process.env[matched[1]] as string)
+      : (userOptions.auth['token'] as string);
+
+    if (!matched) ansToken = ansToken || (userOptions.auth['token'] as string);
+    // matched[1] is env variable
+    if (matched) ansToken = ansToken || (process.env[matched[1]] as string);


Are the last two lines duplicated? We have already assigned ansToken at line 49.

Thanks for catching the part, I have removed it.

oscar60310 · 2022-08-19T03:16:39Z

packages/serve/src/lib/auth/basicAuthenticator.ts

+    const pattern = /^{{([\w]+|[ \w ]+)}}$/;
+    const matched = pattern.exec(userOptions.auth['token'] as string);


It's nice to use environment parameters in property values, but we should do global config rendering instead of replacing them at individual extensions.

We can render our config YAML file with environment parameters, extensions can expect to receive rendered config.

It might also help us do secret masking, context switching ...etc.

Thanks for @oscar60310 suggesting, according to our discussion, I will create the other PR to add it, currently, in this PR I remove environment variable part

oscar60310 · 2022-08-19T03:44:42Z

packages/serve/src/lib/middleware/authMiddleware.ts

+export interface AuthOptions {
+  ['user-auth']?: Array<UserAuthOptions>;
+}


How about moving the per-user config into authenticators' scope?
In our current design, we need to set user information for every user:

auth: options: user-auth: - name: ivan token: some-secret attr: admin: false - name: eason file: pw-file attr: admin: true

It's necessary for us now because we use static user lists, but when we use further auth services like OIDC, we won't want to add users to Vulcan one by one.

So maybe we can change the config like the following (don't mind the property names and their structure):
Using basic auth

auth: options: # For basic auth with password files (we can set attributes at these files too.) password-files: - pw-file # For basic auth with static token, this list is only updated when new user with static token added. static-users: - name: ivan token: some-secret attr: admin: false - name: eason file: pw-file attr: admin: true

Using OIDC (access token)

auth: options: # For OIDC oidc: client-id: xxx client-secret: xxx attribute-mapper: - group

Thanks for @oscar60310 reviewing and suggesting! I make each authenticator option defined by identifier under the options of auth.

auth: options: basic: ... // basic authenticator options simple-token: ... // simple-tokenauthenticator options password-file: ... // password-file authenticator options

So like OIDC, oauth2 could also put under the options of auth.

Thanks alot !

oscar60310 · 2022-08-19T03:47:51Z

packages/serve/src/lib/middleware/authMiddleware.ts

+    // authenticate each user by selected auth method in config
+    for (const name of Object.keys(this.authenticators)) {
+      const result = await this.authenticators[name].authenticate(
+        options['user-auth'] || [],


NIT: I'd prefer inject these options to authenticators directly than passing them here.

Thanks for reviewing and suggesting, after I adapt your suggestion for initializing in onActivate method, I won't pass the options into each authenticate method. Therefore, also no need to inject options.

oscar60310 · 2022-08-19T04:13:25Z

packages/serve/src/lib/middleware/authMiddleware.ts

+      await next();
+      return;
+    }
+    throw new Error('authentication failed.');


Basic authentication should return 401 with www-authenticate header or users can't see the username/password dialog on browsers.
https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/WWW-Authenticate

We need to find a way to throw this error without conflicting with other methods.

Thanks for suggesting, that after I add the multiple auth status for distinguishing what is authentication successful, or failed and "skip to the next authenticator" ( The status called incorrect now ) for checking.

Now I have added the www-authenticate header in response when the authriozation request is basic and options have set basic options data.

oscar60310 · 2022-08-19T04:26:45Z

packages/serve/src/lib/auth/basicAuthenticator.ts

+    for (const userOptions of usersOptions) {
+      if (userOptions.auth['token']) {
+        const result = await this.verifyToken(token, userOptions);
+        if (result.authenticated) return result;
+      }
+      if (userOptions.auth['file']) {
+        const result = await this.verifyTokenInFile(token, userOptions);
+        if (result.authenticated) return result;
+      }
+    }


It'll have better performance if we load the configurations into memory and map them with credentials to the user.

We can call "onActivate" function when middlewares are initializing to load the files and configuration.

We can use Credentials -> Users mapping instead of testing for all users.

public override async onActivate() { // Load config for (const userOptions of usersOptions) { this.credentialMapping.set(userOptions.token, {name, attr ....}) } // Load the password files const reader = readline.createInterface({ input: fs.createReadStream(filePath), }); for await (const line of reader) { this.credentialMapping.set(token, {name, attr ....}) } } private async verifyTokenIn( srcToken: string, ) { const user = this.credentialMapping.get(scrToken); if(!user) { xxxx } return user; }

Thanks for reviewing and suggesting, it's a great suggestion, I have fixed to add the read options in onActivate, thanks !

oscar60310 · 2022-08-19T04:29:26Z

packages/serve/src/lib/auth/basicAuthenticator.ts

+      }
+    }
+
+    if (srcToken !== ansToken) return { authenticated: false };


We can request users to hash their passwords before setting Vulcan like Trino did.

Just want to confirm:

Browsers send basic auth with base64(username + ':' + password), but I haven't seen any decoding process in this file. Does it mean users need to set the correctly base64 encoded password to config?

For example:
base64('test) = dGVzdA==
base64('ivan:test') = aXZhbjp0ZXN0

I need to set user=ivan and token=aXZhbjp0ZXN0 to allow ivan/test to login.

Thanks for reviewing and suggesting!

I will change to Trino suggesting a password format ( bcrypt or PBKDF2 ), currently, seems bcrypt will be great for checking, I think.

And for the question, Does it mean users need to set the correctly base64 encoded password to config? > Yes.
But I seems I miss the checking username part, I will add it.

Thanks for suggesting the hash password idea, I have adopted it in BasicAuthenticator and PasswordFileAuthenticator, each using a different hash way.

For the SimpleTokenAuthetnciator, I make it really simple way for matching whether the value is the same or not.

oscar60310 · 2022-08-19T05:16:28Z

packages/serve/src/lib/auth/basicAuthenticator.ts

+        break;
+      }
+    }
+


We can return here if no ansToken found:

if (ansToken === '') return {authenticated: false};

Thanks for suggesting, I have fixed it!

- support "BaseAuthenticator" for user extending to customize authenticator. - add built-in "HttpBasicAuthenticator" for authenticate http basic token. - refactor type "KoaRouterConext" and "KoaNext". - create "AuthMiddleware" to handle authenticator through implemented authenticators.

- modify the middleware order in "BuiltInRouteMiddlewares". - fix "AuthMiddleware" to set user auth data to correct place in context. - add basic authenticator test cases. - add auth middleware test cases.

- refactor "KeysetBasedStrategy" for passing "keyName" directly. - add cursor, offset, keyset pgination strategy test cases.

…ult status.

…uthenticator.

kokokuo · 2022-08-25T10:40:09Z

Hi @oscar60310, I have fixed all parts from your comment and suggestion! I have also updated the PR description.

Thanks a lot for your reviewing and suggestion!

- update "httpBasicAuthenticator" to change apache-md5 to general md5 for hashing. - refactor to update "httpBasicAuthenticator" test cases. - add "simpleTokenAuthenticator" test cases. - add "passwordFileAuthenticator" test cases. - update "authMiddleware" to activate authenticator and update test cases.

oscar60310

LGTM

oscar60310 · 2022-08-30T02:15:11Z

packages/serve/src/models/extensions/authenticator.ts

+   */
+  SUCCESS = 'SUCCESS',
+  FAIL = 'FAIL',
+  INCORRECT = 'INCORRECT',


NIT: Can we use other words like "indeterminate", "skipped", "bypass" instead of "incorrect"? "Incorrect" makes me think about "bad credentials", and "fail" means failure to authenticate due to internal issues.

Thanks for the suggestion! I chose the indeterminate for renaming,

oscar60310 · 2022-08-30T02:20:03Z

packages/serve/src/lib/middleware/authMiddleware.ts

+
+    this.authenticators = authenticators.reduce<AuthenticatorMap>(
+      (prev, authenticator) => {
+        if (authenticator.activate) authenticator.activate();


Activate functions are asynchronous, we can't wait for them to fulfill in the constructor, maybe we need a activate function for middleware too, and call it from upstream.

Thanks for suggestion, I have refactored it, please check :D

…hen active auth middleware

kokokuo · 2022-08-30T04:31:12Z

Hi @oscar60310 I have fixed the remaining two-part, please check it, thanks so much!

kokokuo marked this pull request as ready for review August 2, 2022 06:51

kokokuo requested a review from oscar60310 August 2, 2022 06:53

Base automatically changed from feature/response-format to develop August 2, 2022 07:32

kokokuo changed the title ~~Feature: Base Authenticator and Built-in Basic Authenticator~~ Feature: Add Auth middleware and Authenticator. Aug 2, 2022

kokokuo force-pushed the feature/authenticator branch from 17b8eee to a22b01b Compare August 17, 2022 06:09

kokokuo changed the base branch from develop to feature/prevent-sql-injection August 17, 2022 06:09

kokokuo force-pushed the feature/authenticator branch from a22b01b to 02a19de Compare August 17, 2022 06:44

kokokuo mentioned this pull request Aug 17, 2022

Feature: Add enforce HTTPS middleware #27

Merged

Base automatically changed from feature/prevent-sql-injection to develop August 18, 2022 01:49

kokokuo force-pushed the feature/authenticator branch 3 times, most recently from 05e1464 to 6b094ef Compare August 19, 2022 01:25

oscar60310 reviewed Aug 19, 2022

View reviewed changes

kokokuo added 6 commits August 25, 2022 16:53

feat(serve): add authenticator and auth middleware test cases

51444cb

- modify the middleware order in "BuiltInRouteMiddlewares". - fix "AuthMiddleware" to set user auth data to correct place in context. - add basic authenticator test cases. - add auth middleware test cases.

feat(serve): add cursor, offset, keyset pgination strategy test cases.

898f0db

- refactor "KeysetBasedStrategy" for passing "keyName" directly. - add cursor, offset, keyset pgination strategy test cases.

fix(serve): make authenticator and auth middleware have different res…

ba1b91f

…ult status.

fix(serve): refactor http basic and add simple-token, password-file a…

8e9c780

…uthenticator.

refactor(serve): add http basic authenticator

a1c8572

kokokuo force-pushed the feature/authenticator branch 4 times, most recently from f780a55 to 9f31a85 Compare August 25, 2022 10:20

kokokuo force-pushed the feature/authenticator branch from 9f31a85 to c95709b Compare August 25, 2022 12:01

oscar60310 approved these changes Aug 30, 2022

View reviewed changes

chore(serve): rename the auth status type, active the authenticator w…

dde285f

…hen active auth middleware

oscar60310 merged commit 658f0f5 into develop Aug 30, 2022

oscar60310 deleted the feature/authenticator branch August 30, 2022 05:55

oscar60310 mentioned this pull request Sep 22, 2022

Fix: Fix dependencies issues #81

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature: Add Auth middleware and Authenticator. #26

Feature: Add Auth middleware and Authenticator. #26

kokokuo commented Aug 2, 2022 •

edited

Loading

kokokuo commented Aug 17, 2022

codecov-commenter commented Aug 19, 2022 •

edited

Loading

oscar60310 left a comment

oscar60310 Aug 18, 2022

kokokuo Aug 25, 2022

oscar60310 Aug 19, 2022

kokokuo Aug 25, 2022

oscar60310 Aug 19, 2022

kokokuo Aug 25, 2022

oscar60310 Aug 19, 2022

kokokuo Aug 25, 2022

oscar60310 Aug 19, 2022

kokokuo Aug 25, 2022

oscar60310 Aug 19, 2022

kokokuo Aug 25, 2022

oscar60310 Aug 19, 2022

kokokuo Aug 25, 2022

oscar60310 Aug 19, 2022

oscar60310 Aug 19, 2022

kokokuo Aug 22, 2022 •

edited

Loading

kokokuo Aug 25, 2022 •

edited

Loading

oscar60310 Aug 19, 2022

kokokuo Aug 25, 2022

kokokuo commented Aug 25, 2022 •

edited

Loading

oscar60310 left a comment

oscar60310 Aug 30, 2022

kokokuo Aug 30, 2022

oscar60310 Aug 30, 2022

kokokuo Aug 30, 2022

kokokuo commented Aug 30, 2022

	@VulcanExtension(TYPES.Extension_Authenticator)
	@VulcanExtension(TYPES.Extension_Authenticator, { enforcedId: true })

		const pattern = /^{{([\w]+\|[ \w ]+)}}$/;
		const matched = pattern.exec(userOptions.auth['token'] as string);

Feature: Add Auth middleware and Authenticator. #26

Feature: Add Auth middleware and Authenticator. #26

Conversation

kokokuo commented Aug 2, 2022 • edited Loading

Description

BasicAuthenticator

SimpleTokenAuthenticator

PasswordFileAuthenticator

How to use it

Sample: doing the auth by the basic authenticator

⚠️ Notice

How To Test / Expected Results

Commit Message

kokokuo commented Aug 17, 2022

codecov-commenter commented Aug 19, 2022 • edited Loading

Codecov Report

oscar60310 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kokokuo Aug 22, 2022 • edited Loading

Choose a reason for hiding this comment

kokokuo Aug 25, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kokokuo commented Aug 25, 2022 • edited Loading

oscar60310 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kokokuo commented Aug 30, 2022

kokokuo commented Aug 2, 2022 •

edited

Loading

codecov-commenter commented Aug 19, 2022 •

edited

Loading

kokokuo Aug 22, 2022 •

edited

Loading

kokokuo Aug 25, 2022 •

edited

Loading

kokokuo commented Aug 25, 2022 •

edited

Loading