feat(security)!: Implementation of JWT authentication ADR by bnevis-i · Pull Request #4244 · edgexfoundry/edgex-go

bnevis-i · 2022-12-03T04:38:12Z

BREAKING CHANGE: Requires JWT authentication for all inbound
requests except for /api/v2/ping URL. Removes support for
Kong reverse proxy. In place of Kong, uses NGINX
proxy auth module and introduces new security-prox-auth service.
Changes secrets-config proxy adduser/deluser commands to create
Vault users instead of Kong user. Changes secrets-config proxy tls
command to write TLS certificate to docker volume instead of Kong.
Removes security-proxy-setup go binary and replaces with shell
script to create default TLS token.

Signed-off-by: Bryon Nevis bryon.nevis@intel.com

PR Checklist

Please check if your PR fulfills the following requirements:

[] I am not introducing a breaking change (if you are, flag in conventional commit message with BREAKING CHANGE: describing the break)
I am not introducing a new dependency (add notes below if you are)
I have added unit tests for the new feature or bug fix (if not, why?)
I have fully tested (add details below) this the new feature or bug fix (if not, why?)
I have opened a PR for the related docs change (if not, why?)

Testing Instructions

New Dependency Instructions (If applicable)

bnevis-i · 2023-03-01T01:41:43Z

@farshidtz To be unblocked by canonical/edgex-snap-testing#163

farshidtz · 2023-03-01T15:05:53Z

@farshidtz To be unblocked by canonical/edgex-snap-testing#163

Thanks, looking into it. But please keep in mind that the snap tests don't block PR merges.

bnevis-i · 2023-03-01T15:43:31Z

@farshidtz To be unblocked by canonical/edgex-snap-testing#163

Thanks, looking into it. But please keep in mind that the snap tests don't block PR merges.

Thanks for the reminder. Probably ok to hold it then.

jim-wang-yutsung

some minor changes

cmd/security-bootstrapper/entrypoint-scripts/nginx_wait_install.sh

cmd/security-bootstrapper/entrypoint-scripts/proxy_setup_wait_install.sh

cmd/security-proxy-setup/Dockerfile

internal/security/config/command/proxy/shared/proxyuser.go

codecov-commenter · 2023-03-02T04:49:55Z

Codecov Report

Merging #4244 (32aa1ba) into main (a219a96) will decrease coverage by 1.89%.
The diff coverage is 60.00%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##             main    #4244      +/-   ##
==========================================
- Coverage   43.66%   41.78%   -1.89%     
==========================================
  Files         116      106      -10     
  Lines       10724     9736     -988     
==========================================
- Hits         4683     4068     -615     
+ Misses       5619     5321     -298     
+ Partials      422      347      -75

Impacted Files	Coverage Δ
...rnal/security/bootstrapper/command/flags_common.go	`0.00% <ø> (ø)`
internal/security/config/command/help/command.go	`100.00% <ø> (ø)`
internal/security/secretstore/init.go	`14.35% <ø> (+0.22%)`	⬆️
...rnal/security/secretstore/secretsengine/enabler.go	`85.18% <0.00%> (ø)`
...l/security/config/command/proxy/deluser/command.go	`71.87% <50.00%> (-1.10%)`	⬇️
...ernal/security/config/command/proxy/tls/command.go	`76.00% <64.28%> (-9.55%)`	⬇️
...l/security/config/command/proxy/adduser/command.go	`69.44% <64.70%> (+5.61%)`	⬆️
...al/security/bootstrapper/command/cmd_dispatcher.go	`100.00% <100.00%> (ø)`

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

internal/security/config/command/proxy/tls/command.go

internal/security/config/bootstraphandler.go

internal/security/config/command/help/command.go

internal/security/config/command/proxy/adduser/command.go

internal/security/config/command/proxy/command.go

internal/security/config/command/proxy/deluser/command.go

BREAKING CHANGE: Requires JWT authentication for all inbound requests except for /api/v2/ping URL. Removes support for Kong reverse proxy. In place of Kong, uses NGINX proxy auth module and introduces new security-prox-auth service. Changes secrets-config proxy adduser/deluser commands to create Vault users instead of Kong user. Changes secrets-config proxy tls command to write TLS certificate to docker volume instead of Kong. Removes security-proxy-setup go binary and replaces with shell script to create default TLS token. Signed-off-by: Bryon Nevis <bryon.nevis@intel.com>

Signed-off-by: Bryon Nevis <bryon.nevis@intel.com>

sonarqubecloud · 2023-03-02T22:07:28Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
2 Code Smells

No Coverage information
5.5% Duplication

jim-wang-yutsung

LGTM

farshidtz

Already merged but I'd like to suggest making the CLI argument naming conventions consistent:

$ edgexfoundry.secrets-config proxy adduser -h
Usage of adduser:
  -configDir string
    	
  -jwtTTL string
    	JWT created by vault identity provider lasts this long (default "1h")
  -tokenTTL string
    	Vault token created as a result of vault login lasts this long (default "1h")
  -useRootToken
    	Set to true to TokenFile in config points to a resp-init.json instead of a service token
  -user string
    	Username of the user to add
level=ERROR ts=2023-03-10T11:55:04.114755814Z app=secrets-config source=bootstraphandler.go:70 msg="unable to parse command: -h: flag: help requested"

Ok - all camelCase

$ edgexfoundry.secrets-config proxy deluser -h
Usage of deluser:
  -configDir string
    	
  -useRootToken
    	Set to true to TokenFile in config points to a resp-init.json instead of a service token
  -user string
    	Username of the user to delete
level=ERROR ts=2023-03-10T11:55:10.795005034Z app=secrets-config source=bootstraphandler.go:70 msg="Unable to parse command: -h: flag: help requested"

Ok - all camelCase

$ edgexfoundry.secrets-config proxy tls -h
Usage of tls:
  -certfilename string
    	Filename of certificate file (on target) (default "nginx.crt")
  -configDir string
    	
  -incert string
    	Path to PEM-encoded leaf certificate
  -inkey string
    	Path to PEM-encoded private key
  -keyfilename string
    	Filename of private key file (on target (default "nginx.key")
  -targetfolder string
    	Path to TLS key file (default "/etc/ssl/nginx")
level=ERROR ts=2023-03-10T11:55:14.547332083Z app=secrets-config source=bootstraphandler.go:70 msg="unable to parse command: -h: flag: help requested"

Not OK.

-certfilename -> -certFilename
-incert -> -inCert
-inkey -> inKey
-keyfilename -> -keyFilename
-targetfolder -> -targetFolder

Also, I don't know why it is printing error on -h.

@bnevis-i I'll open an issue if necessary.

farshidtz · 2023-03-10T12:09:24Z

Related PRs:

bnevis-i · 2023-03-10T18:11:04Z

@farshidtz See #4433 for fixes described in #4244 (review)

bnevis-i changed the title ~~feat(wip): Create secret store tokens via identity auth~~ feat(security)!: Implementation of JWT authentication ADR Feb 8, 2023

bnevis-i mentioned this pull request Mar 1, 2023

feat!: Change identity source for secrets-config proxy adduser/deluser #4389

Closed

5 tasks

bnevis-i marked this pull request as ready for review March 1, 2023 01:10

bnevis-i added this to the Minnesota milestone Mar 1, 2023

bnevis-i mentioned this pull request Mar 1, 2023

feat: Replace check for edgex-kong with security-proxy-auth canonical/edgex-snap-testing#164

Merged

bnevis-i requested review from farshidtz, jim-wang-yutsung and lenny-goodell March 1, 2023 01:42

jim-wang-yutsung requested changes Mar 1, 2023

View reviewed changes

farshidtz mentioned this pull request Mar 1, 2023

feat: Remove check for edgex-kong canonical/edgex-snap-testing#163

Merged

bnevis-i requested a review from jim-wang-yutsung March 2, 2023 17:27

jim-wang-yutsung reviewed Mar 2, 2023

View reviewed changes

bnevis-i added 2 commits March 2, 2023 13:57

feat(security)!: Implementation of JWT authentication ADR

32aa1ba

Signed-off-by: Bryon Nevis <bryon.nevis@intel.com>

bnevis-i requested a review from jim-wang-yutsung March 2, 2023 22:12

jim-wang-yutsung approved these changes Mar 3, 2023

View reviewed changes

cloudxxx8 requested review from chr1shung and removed request for chr1shung March 6, 2023 01:15

bnevis-i merged commit 480818d into edgexfoundry:main Mar 6, 2023

This was referenced Mar 10, 2023

Upgrade secrets-config snap options wrapper canonical/edgex-snap-hooks#81

Closed

Upgrade and enable secrets-config proxy tests canonical/edgex-snap-testing#161

Closed

farshidtz reviewed Mar 10, 2023

View reviewed changes

bnevis-i deleted the vault-identity branch March 10, 2023 17:33

farshidtz mentioned this pull request Mar 14, 2023

refactor(snap): Remove obsolete parts, upgrade Go, build nginx #4439

Merged

5 tasks

Conversation

bnevis-i commented Dec 3, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Checklist

Testing Instructions

New Dependency Instructions (If applicable)

Uh oh!

bnevis-i commented Mar 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

farshidtz commented Mar 1, 2023

Uh oh!

bnevis-i commented Mar 1, 2023

Uh oh!

jim-wang-yutsung left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov-commenter commented Mar 2, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sonarqubecloud bot commented Mar 2, 2023

Uh oh!

jim-wang-yutsung left a comment

Choose a reason for hiding this comment

Uh oh!

farshidtz left a comment

Choose a reason for hiding this comment

Uh oh!

farshidtz commented Mar 10, 2023

Uh oh!

bnevis-i commented Mar 10, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

bnevis-i commented Dec 3, 2022 •

edited

Loading

bnevis-i commented Mar 1, 2023 •

edited

Loading

codecov-commenter commented Mar 2, 2023 •

edited

Loading