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.

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

auth: amazon cognito provider #250

Merged
merged 1 commit into from
Feb 12, 2020
Merged

Conversation

jphines
Copy link
Contributor

@jphines jphines commented Aug 29, 2019

This adds an additional Identity Provider for Amazon Cognito.

There are some TODO's here:

  • Sign out behavior for Amazon Cognito is a bit weird -- there is no endpoint to issues sign-outs except globally.
  • There is no validation endpoint -- we make the full profile call to validate user state
  • Group behavior is a big TODO. It would be nice if we could make this a custom attribute, but something we should look into
  • Documentation

@jphines jphines added enhancement New feature or request in progress labels Aug 29, 2019
@jphines jphines self-assigned this Aug 29, 2019
@codecov
Copy link

codecov bot commented Aug 29, 2019

Codecov Report

Merging #250 into master will decrease coverage by 1.17%.
The diff coverage is 48.85%.

@@            Coverage Diff             @@
##           master     #250      +/-   ##
==========================================
- Coverage   62.53%   61.35%   -1.18%     
==========================================
  Files          54       57       +3     
  Lines        4201     4637     +436     
==========================================
+ Hits         2627     2845     +218     
- Misses       1385     1580     +195     
- Partials      189      212      +23
Impacted Files Coverage Δ
internal/auth/providers/google_mock_admin.go 50% <ø> (ø)
internal/auth/configuration.go 46.97% <0%> (-4.56%) ⬇️
internal/auth/providers/google.go 58.49% <100%> (ø) ⬆️
internal/auth/providers/amazon_cognito_admin.go 22.3% <22.3%> (ø)
...ternal/auth/providers/amazon_cognito_mock_admin.go 33.33% <33.33%> (ø)
internal/auth/providers/amazon_cognito.go 64.31% <64.31%> (ø)
internal/auth/options.go 86.13% <88.88%> (+7.56%) ⬆️

go.mod Outdated Show resolved Hide resolved
@Jusshersmith
Copy link
Contributor

👋 @jphines - pinging you on this as although you wrote the first portion, a fair chunk (and time) has been added since then, so seems worth another general look over, providing you have the bandwidth?

I think the biggest things that have been added/changed is the group membership validation logic (https://github.com/buzzfeed/sso/pull/250/files?file-filters%5B%5D=.go#diff-ea62f132a9d50a2ecbeb99d1dd8c48e1R340),
and the cognito admin file (https://github.com/buzzfeed/sso/pull/250/files?file-filters%5B%5D=.go#diff-c8e9125622d7201b686c3d99338c8e3aR1). Along with some related configuration additions.

@jphines
Copy link
Contributor Author

jphines commented Jan 14, 2020

A few comment nits -- but the logic looks good.

amazon-cognito: init

amazon-cognito: setup

ok

tests

wip sign out

aws sdk

initial commit for group verification feature

some extra tests

empty string slice, not string

additional cognito provider test

extra provider tests

add new tests, and move to options_test.go file

fix cognito validategroup tests

create separate mock admin files

minor edits

remove some comments

couple small improvements

amazon mock admin

changes following review

remove unnecessary method

fixed tests following on from review changes

some updates from review

fix struct name in tests
@Jusshersmith Jusshersmith merged commit c3931bc into master Feb 12, 2020
@Jusshersmith Jusshersmith deleted the amazon-cognito-provider branch February 12, 2020 16:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants