MF-932 - User API keys #941

dborovcanin · 2019-11-08T16:07:01Z

What does this do?

This pull request introduces the API keys service. In the first version, only User keys will be supported.

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

This pull request resolves #932.

List any changes that modify/break current functionality

Creating User token (JWT) is extracted from the Users service in favor of newly introduced API keys service.

Have you included tests for your changes?

Yes.

Did you document any new/modified functionality?

Yes.

codecov-io · 2019-11-26T09:13:17Z

Codecov Report

Merging #941 into master will decrease coverage by 1.03%.
The diff coverage is 90.75%.

@@            Coverage Diff             @@
##           master     #941      +/-   ##
==========================================
- Coverage   83.78%   82.74%   -1.04%     
==========================================
  Files          76       87      +11     
  Lines        5327     5849     +522     
==========================================
+ Hits         4463     4840     +377     
- Misses        588      723     +135     
- Partials      276      286      +10

Impacted Files	Coverage Δ
users/api/transport.go	`83.68% <ø> (ø)`
users/api/requests.go	`73.33% <ø> (ø)`
users/api/endpoint.go	`82.08% <ø> (ø)`
users/api/responses.go	`75% <ø> (ø)`
users/api/metrics.go	`0% <ø> (ø)`
users/api/logging.go	`0% <ø> (ø)`
authn/keys.go	`100% <100%> (ø)`
users/service.go	`86.2% <100%> (+1.59%)`	⬆️
authn/postgres/tracing.go	`100% <100%> (ø)`
authn/api/http/responses.go	`100% <100%> (ø)`
... and 33 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e960004...6dd8c52. Read the comment docs.

manuio · 2019-11-26T19:23:47Z

auth/postgres/init.go

+// unapplied database migrations. A non-nil error is returned to indicate
+// failure.
+func Connect(cfg Config) (*sqlx.DB, error) {
+	url := fmt.Sprintf("host=%s port=%s user=%s dbname=%s password=%s sslmode=%s sslcert=%s sslkey=%s sslrootcert=%s", cfg.Host, cfg.Port, cfg.User, cfg.Name, cfg.Pass, cfg.SSLMode, cfg.SSLCert, cfg.SSLKey, cfg.SSLRootCert)


you can maybe brake this in multiple lines

Maybe using string literals

manuio · 2019-11-26T19:32:32Z

users/api/endpoint_test.go

@@ -350,7 +351,11 @@ func TestPasswordChange(t *testing.T) {
 	ts := newServer(svc)
 	defer ts.Close()
 	client := ts.Client()
-	j := jwt.New("secret")
+	// j := jwt.New("secret")


nmarcetic · 2019-11-28T13:38:11Z

auth/postgres/database.go

+	db *sqlx.DB
+}
+
+// Database provides a database interface


Is it possible to move this to tracing package ? I would expect to see something else in database package, file named database.go

auth/postgres/key.go

drasko · 2019-12-04T22:39:52Z

auth/api/logging.go

+
+func (lm *loggingMiddleware) Identify(ctx context.Context, key string) (id string, err error) {
+	defer func(begin time.Time) {
+		message := fmt.Sprintf("Method identify for took %s to complete", time.Since(begin))


Fix message

drasko · 2019-12-04T22:41:34Z

auth/key.go

+	ErrInvalidKeyIssuedAt = errors.New("invalid issue time")
+
+	// ErrKeyExpired indicates that the Key is expired.
+	ErrKeyExpired = errors.New("use o expired key")


Fix message

drasko · 2019-12-04T22:43:37Z

auth/postgres/key.go

+	"database/sql"
+	"time"
+
+	// required for DB access


Remove useless comment

drasko · 2019-12-04T22:44:10Z

auth/postgres/key.go

+}
+
+// New instantiates a PostgreSQL implementation of key
+// repository.


No need breaking this comment in two lines, especially that lines of code below are longer

Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com>

Simplify tests. Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com>

Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com>

A new name (`tracing.go`) describes better the purpose of the file. Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com>

Fix typo. Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com>

Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com>

Rename token parsing method. Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com>

Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com>

anovakovic01

LGTM!

* Add inital Auth implementation Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com> * Extract IssuedAt on transport layer Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com> * Add token type Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com> * Fix Auth service URL in Things service Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com> * Add User Keys revocation check Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com> * Update tests Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com> * Remove unused tracing methods Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com> * Fix Key retrival and parsing Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com> * Remove unused code Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com> * Increase test coverage Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com> * Fix compose files Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com> * Fix typos Simplify tests. Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com> * Fix typos and remove useless comments Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com> * Rename Auth to Authn Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com> * Rename database.go to tracin.go A new name (`tracing.go`) describes better the purpose of the file. Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com> * Increase test coverage Fix typo. Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com> * Increase test coverage Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com> * Remove token from Users service Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com> * Fix identify login keys Rename token parsing method. Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com> * Extract tokenizer to interface Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com> * Remove pointer time Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com> * Use pointer for expiration time in response Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com> * Use uppercase N Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com> * Remove unnecessary email check Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com> * Cleanup unused code and env vars Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com> * Rename tokenizer field Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com> * Use slices and named fields in test cases Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com> * Update AuthN keys naming Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com> * Remove package-lock.json changes Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com> * Remove Secret from issuing request Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com>

dborovcanin added the work-in-progress label Nov 8, 2019

dborovcanin self-assigned this Nov 8, 2019

dborovcanin force-pushed the MF-932 branch 7 times, most recently from 2f9f6c1 to 9bd9018 Compare November 20, 2019 14:51

dborovcanin force-pushed the MF-932 branch from 813d7b2 to 3239fad Compare November 25, 2019 15:40

dborovcanin force-pushed the MF-932 branch 3 times, most recently from 8b4f9a7 to a3cb563 Compare November 26, 2019 16:25

dborovcanin marked this pull request as ready for review November 26, 2019 18:47

dborovcanin requested a review from a team as a code owner November 26, 2019 18:47

dborovcanin force-pushed the MF-932 branch from 40c74db to 6eb78be Compare November 26, 2019 18:49

dborovcanin removed the work-in-progress label Nov 26, 2019

manuio suggested changes Nov 26, 2019

View reviewed changes

nmarcetic reviewed Nov 28, 2019

View reviewed changes

auth/postgres/key.go Outdated Show resolved Hide resolved

dborovcanin force-pushed the MF-932 branch 4 times, most recently from bae4229 to 76918cb Compare December 4, 2019 17:18

drasko requested changes Dec 4, 2019

View reviewed changes

dborovcanin force-pushed the MF-932 branch 4 times, most recently from 5eadb8d to 6bf9426 Compare December 5, 2019 14:22

dborovcanin added 23 commits December 16, 2019 16:12

Fix Key retrival and parsing

122b05c

Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com>

Remove unused code

6d7ee7f

Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com>

Increase test coverage

c3b1eb8

Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com>

Fix compose files

0803573

Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com>

Fix typos

e8350db

Simplify tests. Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com>

Fix typos and remove useless comments

2e17006

Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com>

Rename Auth to Authn

972a70d

Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com>

Rename database.go to tracin.go

a903d59

A new name (`tracing.go`) describes better the purpose of the file. Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com>

Increase test coverage

d89902a

Fix typo. Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com>

Increase test coverage

a606669

Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com>

Remove token from Users service

0797f2c

Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com>

Fix identify login keys

7f92987

Rename token parsing method. Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com>

Extract tokenizer to interface

cb7a1e4

Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com>

Remove pointer time

6648dd6

Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com>

Use pointer for expiration time in response

e952f23

Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com>

Use uppercase N

fe6de0f

Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com>

Remove unnecessary email check

9f30b98

Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com>

Cleanup unused code and env vars

8a35ea2

Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com>

Rename tokenizer field

be27c8a

Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com>

Use slices and named fields in test cases

1698f4c

Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com>

Update AuthN keys naming

bf9fc53

Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com>

Remove package-lock.json changes

c2410bc

Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com>

Remove Secret from issuing request

50f05c9

Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com>

dborovcanin force-pushed the MF-932 branch from 4a8ad84 to 50f05c9 Compare December 16, 2019 15:13

anovakovic01 approved these changes Dec 16, 2019

View reviewed changes

nmarcetic approved these changes Dec 16, 2019

View reviewed changes

nmarcetic merged commit 9f37927 into absmach:master Dec 16, 2019

nmarcetic mentioned this pull request Dec 18, 2019

Pluggable auth #482

Closed

dborovcanin deleted the MF-932 branch February 17, 2021 09:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MF-932 - User API keys #941

MF-932 - User API keys #941

dborovcanin commented Nov 8, 2019

codecov-io commented Nov 26, 2019 •

edited

Loading

manuio Nov 26, 2019

nmarcetic Nov 28, 2019

manuio Nov 26, 2019

nmarcetic Nov 28, 2019

drasko Dec 4, 2019

drasko Dec 4, 2019

drasko Dec 4, 2019

drasko Dec 4, 2019

anovakovic01 left a comment

MF-932 - User API keys #941

MF-932 - User API keys #941

Conversation

dborovcanin commented Nov 8, 2019

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?

codecov-io commented Nov 26, 2019 • edited Loading

Codecov Report

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anovakovic01 left a comment

Choose a reason for hiding this comment

codecov-io commented Nov 26, 2019 •

edited

Loading