NOISSUE - Add cert revocation to SDK #1693

rodneyosodo · 2023-01-09T11:11:57Z

What does this do?

Fix issue certs SDK by adding BearerPrefix and changing validity to ttl

Which issue(s) does this PR fix/relate to?

Resolves #1700.
Resolves #1697
Closes #1225.

List any changes that modify/break current functionality

N/A

Have you included tests for your changes?

No

Did you document any new/modified functionality?

No

Notes

N/A

manuio · 2023-01-09T12:22:17Z

pkg/sdk/go/certs.go

@@ -34,7 +35,7 @@ func (sdk mfSDK) IssueCert(thingID string, keyBits int, keyType, valid, token st
 	}

 	url := fmt.Sprintf("%s/%s", sdk.certsURL, certsEndpoint)
-	resp, err := request(http.MethodPost, token, url, d)
+	resp, err := request(http.MethodPost, apiutil.BearerPrefix+token, url, d)


Why are you not using sdk.processRequest here?

Updated to used sdk.processRequest

manuio · 2023-01-09T13:42:49Z

pkg/sdk/go/certs.go

-
-	if err := errors.CheckError(resp, http.StatusOK); err != nil {
-		return Cert{}, err
+	_, body, sdkerr := sdk.processRequest(http.MethodPost, url, token, string(CTJSON), d, http.StatusCreated)


You should remove request method and replace it everywhere by sdk.processRequest

codecov-commenter · 2023-01-10T09:53:53Z

Codecov Report

Merging #1693 (a6b3321) into master (b83a344) will increase coverage by 0.06%.
The diff coverage is 12.50%.

@@            Coverage Diff             @@
##           master    #1693      +/-   ##
==========================================
+ Coverage   70.25%   70.32%   +0.06%     
==========================================
  Files         148      148              
  Lines       11512    11501      -11     
==========================================
  Hits         8088     8088              
+ Misses       2757     2745      -12     
- Partials      667      668       +1

Impacted Files	Coverage Δ
pkg/sdk/go/certs.go	`0.00% <0.00%> (ø)`
pkg/sdk/go/responses.go	`0.00% <ø> (ø)`
pkg/sdk/go/sdk.go	`72.30% <ø> (ø)`
certs/service.go	`83.33% <100.00%> (-2.39%)`	⬇️
things/service.go	`66.07% <0.00%> (+0.71%)`	⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Signed-off-by: rodneyosodo <socials@rodneyosodo.com>

drasko

LGTM

arvindh123 · 2023-01-11T16:47:08Z

certs/api/responses.go

@@ -28,6 +28,10 @@ type certsRes struct {
 	created    bool
 }

+type revokeCertsRes struct {
+	RevocationTime time.Time `mapstructure:"revocation_time"`


Do we need the mapstructure tag ?

Signed-off-by: rodneyosodo <socials@rodneyosodo.com>

…x into NOISSUE-clicerts

dborovcanin · 2023-01-11T18:34:14Z

certs/pki/vault.go

 	cReq := certRevokeReq{
-		SerialNumber: serial,
+		SerialNumber: strings.Replace(serial, ":", "-", -1),


Can you add a quick comment explaining this line?

Use http.MethodPost instead of the string in:

r := p.client.NewRequest("POST", p.revokeURL)

Signed-off-by: rodneyosodo <socials@rodneyosodo.com>

drasko

LGTM

rodneyosodo · 2023-01-12T14:49:53Z

Removing key_bits and key_type during issue cert as it raises this error "Endpoint ignored these unrecognized parameters: [key_bits key_type]"

Ref

Signed-off-by: rodneyosodo <socials@rodneyosodo.com>

dborovcanin · 2023-01-13T08:38:05Z

certs/mocks/pki.go

+const (
+	keyBits = 2048
+)


Inline const keyBits = 2048.

dborovcanin · 2023-01-13T08:39:15Z

docker/addons/vault/vault-init.sh

 vault() {
    docker exec -it mainflux-vault vault "$@"
 }

 mkdir -p data

 vault operator init 2>&1 | tee >(sed -r 's/\x1b\[[0-9;]*m//g' > data/secrets)
+
+write_env


Add an empty line.

dborovcanin · 2023-01-13T08:39:35Z

provision/config.go

+	// KeyBits int    `json:"key_bits" toml:"key_bits"`
+	// KeyType string `json:"key_type"`


Remove commented code.

arvindh123 · 2023-01-13T10:07:05Z

provision/service.go

@@ -263,7 +263,7 @@ func (ps *provisionService) Cert(token, thingID, ttl string, keyBits int) (strin
 	if err != nil {


Remove the unused parameter keyBits from the function

Signed-off-by: rodneyosodo <socials@rodneyosodo.com>

dborovcanin · 2023-01-13T10:54:36Z

docker/addons/vault/vault-init.sh

+write_env() {
+    sed -i "s,MF_VAULT_UNSEAL_KEY_1=.*,MF_VAULT_UNSEAL_KEY_1=$(awk -F ": " '$1 == "Unseal Key 1" {print $2}' data/secrets)," $MAINFLUX_DIR/docker/.env
+    sed -i "s,MF_VAULT_UNSEAL_KEY_2=.*,MF_VAULT_UNSEAL_KEY_2=$(awk -F ": " '$1 == "Unseal Key 2" {print $2}' data/secrets)," $MAINFLUX_DIR/docker/.env
+    sed -i "s,MF_VAULT_UNSEAL_KEY_3=.*,MF_VAULT_UNSEAL_KEY_3=$(awk -F ": " '$1 == "Unseal Key 3" {print $2}' data/secrets)," $MAINFLUX_DIR/docker/.env
+    sed -i "s,MF_VAULT_TOKEN=.*,MF_VAULT_TOKEN=$(awk -F ": " '$1 == "Initial Root Token" {print $2}' data/secrets)," $MAINFLUX_DIR/docker/.env
+}


I like this one. :)

drasko

LGTM

arvindh123 · 2023-01-24T04:53:42Z

certs/pki/vault.go

-	resp, err := p.client.RawRequest(r)
-	if resp != nil {
-		defer resp.Body.Close()
+		TTL:        fmt.Sprintf("%sh", ttl),


Do we need to add h prefix to ttl ?
Why the request can we with any prefix like h,m, s

* initial commit Signed-off-by: rodneyosodo <socials@rodneyosodo.com> * fix certificate revoking Signed-off-by: rodneyosodo <socials@rodneyosodo.com> * change from mapstructure to json Signed-off-by: rodneyosodo <socials@rodneyosodo.com> * add comments to serial modification Signed-off-by: rodneyosodo <socials@rodneyosodo.com> * fix typo Signed-off-by: rodneyosodo <socials@rodneyosodo.com> * update vault docker version Signed-off-by: rodneyosodo <socials@rodneyosodo.com> * write env variables Signed-off-by: rodneyosodo <socials@rodneyosodo.com> * change env path Signed-off-by: rodneyosodo <socials@rodneyosodo.com> * return revocation time Signed-off-by: rodneyosodo <socials@rodneyosodo.com> * revert to intermediate CA Signed-off-by: rodneyosodo <socials@rodneyosodo.com> * remove deadcode Signed-off-by: rodneyosodo <socials@rodneyosodo.com> * make revoke cert output readable Signed-off-by: rodneyosodo <socials@rodneyosodo.com> * remove keybits and keytype Signed-off-by: rodneyosodo <socials@rodneyosodo.com> * remove dead code Signed-off-by: rodneyosodo <socials@rodneyosodo.com> * make inline Signed-off-by: rodneyosodo <socials@rodneyosodo.com> * add empty line Signed-off-by: rodneyosodo <socials@rodneyosodo.com> * remove commented code Signed-off-by: rodneyosodo <socials@rodneyosodo.com> * remove keyBits Signed-off-by: rodneyosodo <socials@rodneyosodo.com> * remove keyBits Signed-off-by: rodneyosodo <socials@rodneyosodo.com> Signed-off-by: rodneyosodo <socials@rodneyosodo.com> Co-authored-by: rodneyosodo <socials@rodneyosodo.com> Signed-off-by: rodneyosodo <blackd0t@protonmail.com>

* initial commit Signed-off-by: rodneyosodo <socials@rodneyosodo.com> * fix certificate revoking Signed-off-by: rodneyosodo <socials@rodneyosodo.com> * change from mapstructure to json Signed-off-by: rodneyosodo <socials@rodneyosodo.com> * add comments to serial modification Signed-off-by: rodneyosodo <socials@rodneyosodo.com> * fix typo Signed-off-by: rodneyosodo <socials@rodneyosodo.com> * update vault docker version Signed-off-by: rodneyosodo <socials@rodneyosodo.com> * write env variables Signed-off-by: rodneyosodo <socials@rodneyosodo.com> * change env path Signed-off-by: rodneyosodo <socials@rodneyosodo.com> * return revocation time Signed-off-by: rodneyosodo <socials@rodneyosodo.com> * revert to intermediate CA Signed-off-by: rodneyosodo <socials@rodneyosodo.com> * remove deadcode Signed-off-by: rodneyosodo <socials@rodneyosodo.com> * make revoke cert output readable Signed-off-by: rodneyosodo <socials@rodneyosodo.com> * remove keybits and keytype Signed-off-by: rodneyosodo <socials@rodneyosodo.com> * remove dead code Signed-off-by: rodneyosodo <socials@rodneyosodo.com> * make inline Signed-off-by: rodneyosodo <socials@rodneyosodo.com> * add empty line Signed-off-by: rodneyosodo <socials@rodneyosodo.com> * remove commented code Signed-off-by: rodneyosodo <socials@rodneyosodo.com> * remove keyBits Signed-off-by: rodneyosodo <socials@rodneyosodo.com> * remove keyBits Signed-off-by: rodneyosodo <socials@rodneyosodo.com> Signed-off-by: rodneyosodo <socials@rodneyosodo.com> Co-authored-by: rodneyosodo <socials@rodneyosodo.com>

rodneyosodo requested a review from a team as a code owner January 9, 2023 11:11

manuio reviewed Jan 9, 2023

View reviewed changes

manuio suggested changes Jan 9, 2023

View reviewed changes

dborovcanin changed the title ~~NOISSUE - Fix Issue Certs SDK~~ NOISSUE - Add cert revocation to SDK Jan 10, 2023

rodneyosodo mentioned this pull request Jan 11, 2023

Revoke Certs Failing #1700

Closed

initial commit

71805f7

Signed-off-by: rodneyosodo <socials@rodneyosodo.com>

rodneyosodo force-pushed the NOISSUE-clicerts branch from c99e8bf to 71805f7 Compare January 11, 2023 12:40

rodneyosodo and others added 3 commits January 11, 2023 15:41

Merge branch 'master' into NOISSUE-clicerts

784ebfe

fix certificate revoking

779da4d

Signed-off-by: rodneyosodo <socials@rodneyosodo.com>

Merge branch 'master' into NOISSUE-clicerts

78cb9e4

drasko previously approved these changes Jan 11, 2023

View reviewed changes

arvindh123 reviewed Jan 11, 2023

View reviewed changes

rodneyosodo added 2 commits January 11, 2023 21:33

change from mapstructure to json

6cdef5b

Signed-off-by: rodneyosodo <socials@rodneyosodo.com>

Merge branch 'NOISSUE-clicerts' of 0x.github.com:0x6f736f646f/mainflu…

1f62cff

…x into NOISSUE-clicerts

rodneyosodo dismissed drasko’s stale review via 1f62cff January 11, 2023 18:34

dborovcanin requested changes Jan 11, 2023

View reviewed changes

rodneyosodo added 3 commits January 11, 2023 21:42

add comments to serial modification

97909a2

Signed-off-by: rodneyosodo <socials@rodneyosodo.com>

fix typo

0827ed0

Signed-off-by: rodneyosodo <socials@rodneyosodo.com>

update vault docker version

64bc881

Signed-off-by: rodneyosodo <socials@rodneyosodo.com>

This was referenced Jan 12, 2023

Remove Vault API client from certs service #1225

Closed

Certs PKI - vault client.RawRequest is deprecated #1697

Closed

rodneyosodo added 6 commits January 12, 2023 12:55

write env variables

429b626

Signed-off-by: rodneyosodo <socials@rodneyosodo.com>

change env path

9fdcc5f

Signed-off-by: rodneyosodo <socials@rodneyosodo.com>

return revocation time

9fe6d33

Signed-off-by: rodneyosodo <socials@rodneyosodo.com>

revert to intermediate CA

a1a0dae

Signed-off-by: rodneyosodo <socials@rodneyosodo.com>

remove deadcode

970af29

Signed-off-by: rodneyosodo <socials@rodneyosodo.com>

make revoke cert output readable

f313215

Signed-off-by: rodneyosodo <socials@rodneyosodo.com>

drasko previously approved these changes Jan 12, 2023

View reviewed changes

remove keybits and keytype

0fb45b6

Signed-off-by: rodneyosodo <socials@rodneyosodo.com>

rodneyosodo dismissed drasko’s stale review via 0fb45b6 January 12, 2023 15:11

remove dead code

97c3d6d

Signed-off-by: rodneyosodo <socials@rodneyosodo.com>

dborovcanin requested changes Jan 13, 2023

View reviewed changes

arvindh123 reviewed Jan 13, 2023

View reviewed changes

rodneyosodo added 5 commits January 13, 2023 13:22

make inline

acd8a85

Signed-off-by: rodneyosodo <socials@rodneyosodo.com>

add empty line

cca34c7

Signed-off-by: rodneyosodo <socials@rodneyosodo.com>

remove commented code

b921c87

Signed-off-by: rodneyosodo <socials@rodneyosodo.com>

remove keyBits

add56b6

Signed-off-by: rodneyosodo <socials@rodneyosodo.com>

remove keyBits

a6b3321

Signed-off-by: rodneyosodo <socials@rodneyosodo.com>

dborovcanin approved these changes Jan 13, 2023

View reviewed changes

drasko approved these changes Jan 13, 2023

View reviewed changes

drasko merged commit d008ae5 into absmach:master Jan 13, 2023

arvindh123 reviewed Jan 24, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

NOISSUE - Add cert revocation to SDK #1693

NOISSUE - Add cert revocation to SDK #1693

rodneyosodo commented Jan 9, 2023 •

edited

Loading

manuio Jan 9, 2023

rodneyosodo Jan 9, 2023

manuio Jan 9, 2023

rodneyosodo Jan 10, 2023

codecov-commenter commented Jan 10, 2023 •

edited

Loading

drasko left a comment

arvindh123 Jan 11, 2023

rodneyosodo Jan 11, 2023

dborovcanin Jan 11, 2023

dborovcanin Jan 11, 2023

rodneyosodo Jan 11, 2023

drasko left a comment

rodneyosodo commented Jan 12, 2023

dborovcanin Jan 13, 2023

dborovcanin Jan 13, 2023

dborovcanin Jan 13, 2023

arvindh123 Jan 13, 2023 •

edited

Loading

dborovcanin Jan 13, 2023

arvindh123 Jan 13, 2023

drasko left a comment

arvindh123 Jan 24, 2023

		// KeyBits int `json:"key_bits" toml:"key_bits"`
		// KeyType string `json:"key_type"`

		@@ -263,7 +263,7 @@ func (ps *provisionService) Cert(token, thingID, ttl string, keyBits int) (strin
		if err != nil {

NOISSUE - Add cert revocation to SDK #1693

NOISSUE - Add cert revocation to SDK #1693

Conversation

rodneyosodo commented Jan 9, 2023 • edited Loading

What does this do?

Which issue(s) does this PR fix/relate to?

List any changes that modify/break current functionality

Have you included tests for your changes?

Did you document any new/modified functionality?

Notes

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

codecov-commenter commented Jan 10, 2023 • edited Loading

Codecov Report

drasko 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

drasko left a comment

Choose a reason for hiding this comment

rodneyosodo commented Jan 12, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arvindh123 Jan 13, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

drasko left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rodneyosodo commented Jan 9, 2023 •

edited

Loading

codecov-commenter commented Jan 10, 2023 •

edited

Loading

arvindh123 Jan 13, 2023 •

edited

Loading