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.

Sign up for GitHub

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

Jump to bottom

feat: "kid" support in JWT header and multiple keys. #392

Merged
merged 1 commit into from
Nov 29, 2023
Merged

feat: "kid" support in JWT header and multiple keys. #392

merged 1 commit into from
Nov 29, 2023

Conversation

merlin-northern
Copy link
Contributor

Ticket: MEN-6804
Changelog: title

@mender-test-bot
Copy link

@merlin-northern, Let me know if you want to start the integration pipeline by mentioning me and the command "start pipeline".

my commands and options

You can trigger a pipeline on multiple prs with:

  • mentioning me and start pipeline --pr mender/127 --pr mender-connect/255

You can start a fast pipeline, disabling full integration tests with:

  • mentioning me and start pipeline --fast

You can trigger GitHub->GitLab branch sync with:

  • mentioning me and sync

You can cherry pick to a given branch or branches with:

  • mentioning me and:
 cherry-pick to:
 * 1.0.x
 * 2.0.x

@merlin-northern merlin-northern changed the title [work-in-progress] feat: "kid" support in JWT header and multiple keys. feat: "kid" support in JWT header and multiple keys. Nov 17, 2023
@merlin-northern merlin-northern force-pushed the men_6804_key_rotation_support branch 9 times, most recently from fff601c to fcfc84f Compare November 17, 2023 12:28
@merlin-northern
Copy link
Contributor Author

@mender-test-bot start pipeline please honey

@mender-test-bot
Copy link

Hello 😸 I created a pipeline for you here: Pipeline-1076295007

Build Configuration Matrix

Key Value
AUDITLOGS_REV master
BUILD_CLIENT false
CREATE_ARTIFACT_WORKER_REV master
DEPLOYMENTS_ENTERPRISE_REV master
DEPLOYMENTS_REV master
DEVICEAUTH_ENTERPRISE_REV master
DEVICEAUTH_REV master
DEVICECONFIG_REV master
DEVICECONNECT_REV master
DEVICEMONITOR_REV master
GENERATE_DELTA_WORKER_REV master
GUI_REV master
INTEGRATION_REV master
INVENTORY_ENTERPRISE_REV master
INVENTORY_REV master
IOT_MANAGER_REV master
MENDER_ARTIFACT_REV master
MENDER_BINARY_DELTA_REV master
MENDER_CLI_REV master
MENDER_CONFIGURE_MODULE_REV master
MENDER_CONNECT_REV master
MENDER_CONVERT_REV master
MENDER_GATEWAY_REV master
MENDER_REV master
MENDER_SETUP_REV master
MENDER_SNAPSHOT_REV master
MONITOR_CLIENT_REV master
MTLS_AMBASSADOR_REV master
RUN_BACKEND_INTEGRATION_TESTS true
RUN_INTEGRATION_TESTS true
TENANTADM_REV master
USERADM_ENTERPRISE_REV master
USERADM_REV pull/392/head
WORKFLOWS_ENTERPRISE_REV master
WORKFLOWS_REV master

kjaskiewiczz
kjaskiewiczz reviewed Nov 20, 2023
View reviewed changes
authz/middleware.go Outdated Show resolved Hide resolved
jwt/jwt_ed25519.go Outdated Show resolved Hide resolved
jwt/jwt_rsa.go Outdated Show resolved Hide resolved
server.go Outdated Show resolved Hide resolved
user/useradm.go Outdated Show resolved Hide resolved
jwt/token.go Show resolved Hide resolved
alfrunes
alfrunes reviewed Nov 20, 2023
View reviewed changes
Copy link
Contributor

@alfrunes alfrunes left a comment

Choose a reason for hiding this comment

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

The title of the PR and the ticket description does not align.
Although I like the idea of adding support for JWK support in the backend, the ticket was actually about adding support for allowing tokens with no key signature. Probably the easiest way of achieving this is to set the alg header field to none if none of the private key configs are set.

@merlin-northern
Copy link
Contributor Author

merlin-northern commented Nov 20, 2023 •
edited
Loading

The title of the PR and the ticket description does not align. Although I like the idea of adding support for JWK support in the backend, the ticket was actually about adding support for allowing tokens with no key signature. Probably the easiest way of achieving this is to set the alg header field to none if none of the private key configs are set.

thanks. if you like it, I consider it a good sign :) and I have been down that road, the library we use will not allow to verify the token with alg none.

@merlin-northern merlin-northern force-pushed the men_6804_key_rotation_support branch 2 times, most recently from ad687a3 to 949c990 Compare November 21, 2023 06:46
kjaskiewiczz
kjaskiewiczz approved these changes Nov 21, 2023
View reviewed changes
Copy link
Contributor

@kjaskiewiczz kjaskiewiczz left a comment

Choose a reason for hiding this comment

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

looks good to me, but since it's not exactly what was requested in the ticket, I'd like to know @alfrunes (the author of the ticket) opinion before merging

@merlin-northern
Copy link
Contributor Author

looks good to me, but since it's not exactly what was requested in the ticket, I'd like to know @alfrunes (the author of the ticket) opinion before merging

no worries, there is already an item added for tomorrow.

@merlin-northern
Copy link
Contributor Author

@mender-test-bot start pipeline please honey

@mender-test-bot
Copy link

Hello 😸 I created a pipeline for you here: Pipeline-1088486974

Build Configuration Matrix

Key Value
AUDITLOGS_REV master
BUILD_CLIENT false
CREATE_ARTIFACT_WORKER_REV master
DEPLOYMENTS_ENTERPRISE_REV master
DEPLOYMENTS_REV master
DEVICEAUTH_ENTERPRISE_REV master
DEVICEAUTH_REV master
DEVICECONFIG_REV master
DEVICECONNECT_REV master
DEVICEMONITOR_REV master
GENERATE_DELTA_WORKER_REV master
GUI_REV master
INTEGRATION_REV master
INVENTORY_ENTERPRISE_REV master
INVENTORY_REV master
IOT_MANAGER_REV master
MENDER_ARTIFACT_REV master
MENDER_BINARY_DELTA_REV master
MENDER_CLI_REV master
MENDER_CONFIGURE_MODULE_REV master
MENDER_CONNECT_REV master
MENDER_CONVERT_REV master
MENDER_GATEWAY_REV master
MENDER_REV master
MENDER_SETUP_REV master
MENDER_SNAPSHOT_REV master
MONITOR_CLIENT_REV master
MTLS_AMBASSADOR_REV master
RUN_BACKEND_INTEGRATION_TESTS true
RUN_INTEGRATION_TESTS true
TENANTADM_REV master
USERADM_ENTERPRISE_REV master
USERADM_REV pull/392/head
WORKFLOWS_ENTERPRISE_REV master
WORKFLOWS_REV master

kjaskiewiczz
kjaskiewiczz reviewed Nov 29, 2023
View reviewed changes
rsa.in Outdated Show resolved Hide resolved
kjaskiewiczz
kjaskiewiczz reviewed Nov 29, 2023
View reviewed changes
ed25519.in Outdated Show resolved Hide resolved
@merlin-northern
feat: "kid" support in JWT header and multiple keys.
27467e3 
Ticket: MEN-6804
Changelog: title
Signed-off-by: Peter Grzybowski <[email protected]>
@mender-test-bot
Copy link

mender-test-bot commented Nov 29, 2023 •
edited by jira bot
Loading

Merging these commits will result in the following changelog entries:

Changelogs

useradm (men_6804_key_rotation_support)

New changes in useradm since master:

Features
  • "kid" support in JWT header and multiple keys.
    (MEN-6804)

@merlin-northern
Copy link
Contributor Author

@mender-test-bot start pipeline --fast please

@mender-test-bot
Copy link

Hello 😸 I created a pipeline for you here: Pipeline-1089697069

Build Configuration Matrix

Key Value
AUDITLOGS_REV master
BUILD_CLIENT false
CREATE_ARTIFACT_WORKER_REV master
DEPLOYMENTS_ENTERPRISE_REV master
DEPLOYMENTS_REV master
DEVICEAUTH_ENTERPRISE_REV master
DEVICEAUTH_REV master
DEVICECONFIG_REV master
DEVICECONNECT_REV master
DEVICEMONITOR_REV master
GENERATE_DELTA_WORKER_REV master
GUI_REV master
INTEGRATION_REV master
INVENTORY_ENTERPRISE_REV master
INVENTORY_REV master
IOT_MANAGER_REV master
MENDER_ARTIFACT_REV master
MENDER_BINARY_DELTA_REV master
MENDER_CLI_REV master
MENDER_CONFIGURE_MODULE_REV master
MENDER_CONNECT_REV master
MENDER_CONVERT_REV master
MENDER_GATEWAY_REV master
MENDER_REV master
MENDER_SETUP_REV master
MENDER_SNAPSHOT_REV master
MONITOR_CLIENT_REV master
MTLS_AMBASSADOR_REV master
RUN_BACKEND_INTEGRATION_TESTS true
RUN_INTEGRATION_TESTS false
TENANTADM_REV master
USERADM_ENTERPRISE_REV master
USERADM_REV pull/392/head
WORKFLOWS_ENTERPRISE_REV master
WORKFLOWS_REV master

@merlin-northern merlin-northern merged commit e77736d into mendersoftware:master Nov 29, 2023
10 checks passed
@mender-test-bot
Copy link

Hello 😸 This PR contains changelog entries. Please, verify the need of backporting it to the following release branches:
1.21.x (release 3.6.x)
1.18.x (release 3.3.x)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kjaskiewiczz kjaskiewiczz kjaskiewiczz approved these changes

@alfrunes alfrunes alfrunes left review comments

@tranchitella tranchitella Awaiting requested review from tranchitella

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@merlin-northern @mender-test-bot @kjaskiewiczz @alfrunes