Add JWT issuing and validation. Change Harvester onboarding and authentication processes. #151
Conversation
maxlambrecht
requested review from
wibarre,
mchurichi,
jufantozzi,
mgbcaio and
Victorblsilveira
as code owners
maxlambrecht
force-pushed
the
feature/jwt-issuer-validator
branch
from
790e930 to
59ca42d
Compare
|
WIP: unit tests for the |
There was a problem hiding this comment.
Excellent work here @maxlambrecht really good changes. I have just some minor doubts.|
See if make sense.
There was a problem hiding this comment.
I left 1 nit comment. So feel free to skip it if necessary.
All good, good job @maxlambrecht
Thanks @Victorblsilveira. |
Signed-off-by: Max Lambrecht <[email protected]>
Signed-off-by: Max Lambrecht <[email protected]>
Signed-off-by: Max Lambrecht <[email protected]>
Signed-off-by: Max Lambrecht <[email protected]>
Signed-off-by: Max Lambrecht <[email protected]>
maxlambrecht
force-pushed
the
feature/jwt-issuer-validator
branch
from
1e0fbf3 to
273e5a4
Compare
Signed-off-by: Max Lambrecht <[email protected]>
Signed-off-by: Max Lambrecht <[email protected]>
Signed-off-by: Max Lambrecht <[email protected]>
|
@Victorblsilveira I refactored the Onboard and GetNewJWTToken requests using a method |
Signed-off-by: Max Lambrecht <[email protected]>
|
SonarCloud Quality Gate failed.
|
Signed-off-by: Max Lambrecht <[email protected]>
I just realized that yesterday, on my bed, when trying to sleep. Why don't we use the client boilerplate for making that calls? |
| assert.NotNil(t, key1) | ||
| assert.Equal(t, "foo", key1.ID()) | ||
|
|
||
| key2, err := km.GenerateKey(ctx, "bar", cryptoutil.RSA2048) |
There was a problem hiding this comment.
| key2, err := km.GenerateKey(ctx, "bar", cryptoutil.RSA2048) | |
| key2, err := km.GenerateKey(ctx, "bar", cryptoutil.RSA4096) |
is this suppposed to be like this ?
There was a problem hiding this comment.
No, what matters is the key id "bar".
| if err != nil { | ||
| err := fmt.Errorf("failed to read bundle put body: %v", err) | ||
| return h.handleErrorAndLog(err, http.StatusBadRequest) | ||
| return h.handleErrorAndLog(err, err, http.StatusBadRequest) |
There was a problem hiding this comment.
Why do we need to pass the err multiple times, cant the function use the same parameter ?
There was a problem hiding this comment.
Because the method both logs the error and handles the error sending back a message to the caller, thus it takes in an error to log and an error to return, that usually are not the same.
That's a good question. I guess we can use the generated client. We can do the refactor in another PR. |
Pull request check list
Affected functionality
Changed the harvester onboarding and authentication process:
The onboarding process uses a
join_token, validates it's still valid and not used, and returns a JWT token with the subject equal to the onboarded trust domain name.The authentication of API calls is done using a JWT validator to verify the Bearer token from the Authorization header.
Description of change
Added interfaces
jwt.Issuer,jwt.Validator, andKeyManager, to issue and validate JWT tokens, using keys managed by aKeyManager.The implementation added for the
KeyManagerinterface is a simple in-memory manager.Added a goroutine that request a new JWT token every 12 hours.
Added decorator to every API call in the Harvester to add the JWT Bearer token.
Which issue this pull requests fixes
Fixes GLCP-45111