Custom Authentication Plugin #1314

tom-stclair · 2021-12-15T18:28:48Z

Description

Added the ability to inject a custom authentication strategy using a Serverless plugin

Motivation and Context

I have the need for an authentication strategy that isn't satisfied by the existing capabilities of the project. This strategy is also very specific and not useful as a strategy to include in the project itself. This change allows the underlying authentication strategy used by the HttpServer.js to be defined in an external Serverless plugin.

How Has This Been Tested?

A new integration test has been added to the project that verifies that the authentication can be injected through a plugin.
All tests for the project have been run via the existing npm run test.
This branch has also been tested locally by incorporating it into existing projects that depend on the Serverless offline project.

Screenshots (if appropriate):

Added the ability to inject a custom authentication strategy using a Serverless plugin

medikoo

Thank you @tom-stclair

It looks very promising. Please see my comments

@pgrzesik what do you think of this addition?

src/events/http/HttpServer.js

CHANGELOG.md

package-lock.json

package.json

removing package-lock.json

tom-stclair · 2021-12-16T15:29:32Z

@medikoo,
If there is a desire to keep the package-lock.json out of the project's repo, how do you feel about me updating the .gitignore file with this PR so this doesn't happen to others?

Regarding version number changes to the package.json and CHANGELOG.md made by contributors, what if I added a statement in the CONTRIBUTING.md with a statement that indicates that these changes shouldn't be included in PRs?

medikoo · 2021-12-16T15:42:57Z

@medikoo,
If there is a desire to keep the package-lock.json out of the project's repo, how do you feel about me updating the .gitignore file with this PR so this doesn't happen to others?

It's a great idea. Still let's propose it with different PR

xr0master · 2021-12-18T22:13:55Z

@medikoo https://docs.npmjs.com/cli/v8/configuring-npm/package-lock-json

package-lock.json is automatically generated for any operations where npm modifies either the node_modules tree, or package.json...
This file is intended to be committed into source repositories, and serves various purposes...

medikoo · 2021-12-20T12:21:32Z

@xr0master I'm aware of this note. Still in case of this library, keeping this file is just about extra maintenance friction. with no real value added.

Note it doesn't play any role for plugin users, and when maintenance is concerned we prefer the plugin to be tested in a similar environment in which it's actually used. For that, we intentionally ensure all deps are installed at the latest version to CI, so eventual regressions as caused by dependencies are detected early (with package-lock.json on board it will not be possible)

xr0master · 2021-12-20T14:26:21Z

@medikoo Curiously, in this case, you're right. So, all dependencies must also have a caret (^)...
Thanks for the clarification.

tom-stclair · 2022-01-03T13:48:14Z

@medikoo @pgrzesik
Any chance to move this forward or provide more feedback?

pgrzesik · 2022-01-05T16:16:19Z

Sorry @tom-stclair and @medikoo for not responding sooner, I've missed previous mention. I think this definitely sounds interesting, not sure how much people would be interested in using this functionality, but I don't see a reason as to why we shouldn't accept it.

Just of curiosity @tom-stclair - is custom auth the only way to achieve it in your case? Maybe the plugin is missing a certain authentication strategy that should be just implemented directly in the plugin?

tom-stclair · 2022-01-06T14:23:10Z

@pgrzesik
From what we could tell the functionality we wanted wasn’t available, but it’s possible we may have overlooked something. The specific implementation of our authentication plugin is unlikely to have broad utility. The software we are building and using this project in supports multiple authentication tokens, JWTs, and custom keys. We normalize the claims at the API Gateway into a very specific representation, where software downstream is abstracted away from the variability of different types of tokens.

The custom authentication plugin we are looking to inject with this change allows us to declare the desired normalized claims that are provided to the invoked functions and greatly simplifies our ability to test and validate our software. It seemed reasonable that others using this project will have special authentication needs and rather than trying to update the project to satisfy every case a better approach was to allow this aspect to be extensible using the plugin framework of the broader serverless package. I see this change as having the potential to relieve pressure off of the maintainers of this project while also enabling consumers to move forward more rapidly.

Please let me know if you think we are overlooking any existing functionality or you want more detail on our specific use case touched on above.

pgrzesik · 2022-01-06T15:56:41Z

Thanks for clarification @tom-stclair - that makes sense, @medikoo do you think we should move forward with this? I think we can have that in

medikoo

@pgrzesik thanks for feedback

@tom-stclair we're definitely fine to go with it, but I see one potential issue with proposed approach, please see my comment

medikoo · 2022-01-06T17:13:35Z

tests/integration/custom-authentication/customPlugin.js

+      },
+    }
+
+    this.options.plugins = {


Doing that will change globally options object for all plugins and serverless internals.

Any modifications to it are already coined as an anti-pattern: serverless/serverless#2582

It would be nicer to attach this configuration not via plugin, but simply via external module, something close to:

serverless.yml

custom: offline: customAuthorizationStrategy: ./authorization-strategy.js

authorization-strategy.js

module.exports = (endpoint, functionKey, method, path) => { ... }

@medikoo Thanks for the advice and apologies for the late reply. I believe I understand your suggestion, I need a bit of time to look into how serverless custom attributes are managed and how to inject the desired behavior from an external package.

Time has gone by quickly since your last reply and I wanted you to know that the intention is still to follow through with the suggested changes. It will be a few more days before I get a chance to get this updated.

Thanks again

Great thanks @tom-stclair for heads up. No worries, we look forward the update!

@medikoo - had a chance to make the suggested changes. Let me know if you had something else in mind.

Updated the way custom authentication can be injected. Moved from a serverless plugin to a custom variable in the serverless.yml 'offline.customAuthenticationProvider'

tom-stclair · 2022-02-07T14:37:56Z

src/events/http/HttpServer.js

+      customizations.offline?.customAuthenticationProvider
+    ) {
+      // eslint-disable-next-line import/no-dynamic-require, global-require
+      const provider = require(customizations.offline


@medikoo - Based on your earlier feedback on moving this away from a serverless plugin, let me know if you see a better way to inject this. My desire would be that in serverless.yml you'd be able to add just a regular import statement in
custom: offline: customAuthenticationProvider:
this makes for an inline 'require', unless you know of a better way?

medikoo · 2022-02-08T09:12:43Z

src/events/http/HttpServer.js

+      customizations.offline?.customAuthenticationProvider
+    ) {
+      // eslint-disable-next-line import/no-dynamic-require, global-require
+      const provider = require(customizations.offline


This require will search in context of a directory in which this HttpServer.js file is placed. While ideally we should search against service directory (it's also how we load service local plugins).

For that you can rely on https://nodejs.org/api/module.html#modulecreaterequirefilename, as we do indirectly in Framework here: https://github.com/serverless/serverless/blob/480d74ec58c78466d2d055e5c2d73c2f4dde0b42/lib/classes/plugin-manager.js#L159

@medikoo - thanks for the feedback. I knew that the search would be relative to the HttpServer and wasn't thrilled with the confusion this could create but I didn't know this could be modified via createRequire.

I think this matches your suggestions, let me know if it doesn't.

moved from require() to createRequire() for custom authentication provider imports. This provides a more intuitive root directory for relative imports from the custom variables.

medikoo

Thank you @tom-stclair ! Looks good to me. Great improvement

medikoo · 2022-02-10T09:50:08Z

@tom-stclair can you rebase against master?

tom-stclair · 2022-02-10T14:01:17Z

@medikoo - brought current with master

medikoo · 2022-02-11T08:59:23Z

@tom-stclair it's still marked as conflicting

Added the ability to inject a custom authentication strategy using custom variables in the serverless definition, see README.md for more details.

removing package-lock.json

Updated the way custom authentication can be injected. Moved from a serverless plugin to a custom variable in the serverless.yml 'offline.customAuthenticationProvider'

moved from require() to createRequire() for custom authentication provider imports. This provides a more intuitive root directory for relative imports from the custom variables.

…serverless-offline into auth-plugin-support

tom-stclair · 2022-02-11T15:07:12Z

@medikoo - I rebased with master, do you still see conflicts?

medikoo · 2022-02-11T15:59:04Z

@tom-stclair GitHub still reports as below, still I see I can merge it via squash'ing, I'll do it then

tom-stclair · 2022-02-11T17:24:06Z

@medikoo - that is excellent news about squash and merge, this seems to be a much simpler approach and I apprecienate it. Thanks for everything.

tom-stclair · 2022-02-17T17:46:36Z

@medikoo - What does the release schedule look like for this project, wondering when I can expect to see a new NPM package with this change?

Thanks

pgrzesik · 2022-02-17T19:19:01Z

Hey @tom-stclair - I will prepare a release tomorrow

Custom Authentication Plugin

79c1f3b

Added the ability to inject a custom authentication strategy using a Serverless plugin

medikoo reviewed Dec 16, 2021

View reviewed changes

src/events/http/HttpServer.js Show resolved Hide resolved

CHANGELOG.md Outdated Show resolved Hide resolved

package-lock.json Outdated Show resolved Hide resolved

package.json Outdated Show resolved Hide resolved

tom-stclair added 2 commits December 16, 2021 06:46

Delete package-lock.json

18ec725

removing package-lock.json

removing version number updates

88a59f0

medikoo reviewed Jan 6, 2022

View reviewed changes

This was referenced Jan 17, 2022

[Snyk] Security upgrade node-fetch from 2.6.0 to 3.1.1 Itchibon777/serverless-offline#32

Open

[Snyk] Security upgrade node-fetch from 2.6.0 to 3.1.1 Itchibon777/serverless-offline#33

Open

Moved from plugin to custom variable

910317e

Updated the way custom authentication can be injected. Moved from a serverless plugin to a custom variable in the serverless.yml 'offline.customAuthenticationProvider'

tom-stclair commented Feb 7, 2022

View reviewed changes

medikoo reviewed Feb 8, 2022

View reviewed changes

changed import process for custom authentication provider

a6aefdd

moved from require() to createRequire() for custom authentication provider imports. This provides a more intuitive root directory for relative imports from the custom variables.

medikoo approved these changes Feb 10, 2022

View reviewed changes

Merge branch 'master' into auth-plugin-support

24802cc

tom-stclair added 5 commits February 11, 2022 06:44

Updated README.md via npm run prettify

fa92088

Custom Authentication Plugin

0a7b359

Added the ability to inject a custom authentication strategy using custom variables in the serverless definition, see README.md for more details.

Delete package-lock.json

698daa2

removing package-lock.json

removing version number updates

a5abf60

Moved from plugin to custom variable

ea7fc2a

Updated the way custom authentication can be injected. Moved from a serverless plugin to a custom variable in the serverless.yml 'offline.customAuthenticationProvider'

tom-stclair added 4 commits February 11, 2022 07:47

changed import process for custom authentication provider

8a9e4b3

moved from require() to createRequire() for custom authentication provider imports. This provides a more intuitive root directory for relative imports from the custom variables.

Updated README.md via npm run prettify

b807139

Merge branch 'auth-plugin-support' of https://github.com/tom-stclair/…

bf4dbf0

…serverless-offline into auth-plugin-support

Updated package.json to expected version

f7dd53c

medikoo merged commit febfe77 into dherault:master Feb 11, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Custom Authentication Plugin #1314

Custom Authentication Plugin #1314

tom-stclair commented Dec 15, 2021

medikoo left a comment

tom-stclair commented Dec 16, 2021

medikoo commented Dec 16, 2021

xr0master commented Dec 18, 2021

medikoo commented Dec 20, 2021

xr0master commented Dec 20, 2021

tom-stclair commented Jan 3, 2022

pgrzesik commented Jan 5, 2022

tom-stclair commented Jan 6, 2022

pgrzesik commented Jan 6, 2022

medikoo left a comment

medikoo Jan 6, 2022

tom-stclair Jan 25, 2022

medikoo Jan 26, 2022

tom-stclair Feb 7, 2022

tom-stclair Feb 7, 2022

medikoo Feb 8, 2022

tom-stclair Feb 9, 2022

medikoo left a comment

medikoo commented Feb 10, 2022

tom-stclair commented Feb 10, 2022

medikoo commented Feb 11, 2022

tom-stclair commented Feb 11, 2022

medikoo commented Feb 11, 2022

tom-stclair commented Feb 11, 2022

tom-stclair commented Feb 17, 2022

pgrzesik commented Feb 17, 2022

Custom Authentication Plugin #1314

Custom Authentication Plugin #1314

Conversation

tom-stclair commented Dec 15, 2021

Description

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

medikoo left a comment

Choose a reason for hiding this comment

tom-stclair commented Dec 16, 2021

medikoo commented Dec 16, 2021

xr0master commented Dec 18, 2021

medikoo commented Dec 20, 2021

xr0master commented Dec 20, 2021

tom-stclair commented Jan 3, 2022

pgrzesik commented Jan 5, 2022

tom-stclair commented Jan 6, 2022

pgrzesik commented Jan 6, 2022

medikoo left a comment

Choose a reason for hiding this comment

medikoo Jan 6, 2022

Choose a reason for hiding this comment

tom-stclair Jan 25, 2022

Choose a reason for hiding this comment

medikoo Jan 26, 2022

Choose a reason for hiding this comment

tom-stclair Feb 7, 2022

Choose a reason for hiding this comment

tom-stclair Feb 7, 2022

Choose a reason for hiding this comment

medikoo Feb 8, 2022

Choose a reason for hiding this comment

tom-stclair Feb 9, 2022

Choose a reason for hiding this comment

medikoo left a comment

Choose a reason for hiding this comment

medikoo commented Feb 10, 2022

tom-stclair commented Feb 10, 2022

medikoo commented Feb 11, 2022

tom-stclair commented Feb 11, 2022

medikoo commented Feb 11, 2022

tom-stclair commented Feb 11, 2022

tom-stclair commented Feb 17, 2022

pgrzesik commented Feb 17, 2022