Add an IdpGetAll endpoint, return meta-url and login-url #399
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
fisx
reviewed
Jul 10, 2018
services/spar/test/Arbitrary.hs
Outdated
| instance Arbitrary IdPList where | ||
| arbitrary = do | ||
| _idplProviders <- arbitrary | ||
| pure $ IdPList {..} |
There was a problem hiding this comment.
arbitrary = IdPList <$> arbitrarybut it's both good.
fisx
reviewed
Jul 10, 2018
services/spar/spar.integration.yaml
Outdated
| @@ -14,6 +14,9 @@ saml: | |||
| surname: Girlfriend | |||
| email: email:president@evil.corp | |||
|
|
|||
| metaUrl: https://app.wire.com/sso/metadata # corresponds to 'APIMeta' | |||
There was a problem hiding this comment.
Should be under spInfo. There are at least two metadata urls, one for IdP, one for SP, and it's not clear from the name here which is meant.
Also, should the comment be in the source code? That's what it refers to.
|
I will continue review later, and probably make the changes to I can merge myself if you have no objections. Looking really good so far, thanks! |
|
I 140% don't mind any changes
…On Tue, Jul 10, 2018, 17:47 fisx ***@***.***> wrote:
I will continue review later, and probably make the changes to I can merge
myself if you have no objections.
Looking really good so far, thanks!
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#399 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABc-ag_WyaiI9ug6EeOkgGQeVpsszzIaks5uFMyBgaJpZM4VJlOa>
.
|
|
|
||
| -- | A list of 'IdP's, returned by some endpoints. Wrapped into an object to | ||
| -- allow extensibility later on. | ||
| data IdPList = IdPList |
There was a problem hiding this comment.
I think that similarly to #402, we should make this an object
There was a problem hiding this comment.
it already is:
*Spar.Types Aaeson> encode (IdPList [])
"{\"providers\":[]}"
|
Isn't it already one? If not, it should be, yes
…On Thu, Jul 12, 2018, 10:14 Tiago Manuel Ventura Loureiro < ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In services/spar/src/Spar/Types.hs
<#399 (comment)>:
> +type IdP = IdPConfig IdPExtra
+
+-- | Extra information stored for each 'IdP'. The SAML library handles it
+-- but never inspects it (see the '_idpExtraInfo' field).
+data IdPExtra = IdPExtra
+ { _idpeTeam :: TeamId
+ , _idpeSPInfo :: SPInfo
+ }
+ deriving (Eq, Show, Generic)
+
+makeLenses ''IdPExtra
+deriveJSON deriveJSONOptions ''IdPExtra
+
+-- | A list of 'IdP's, returned by some endpoints. Wrapped into an object to
+-- allow extensibility later on.
+data IdPList = IdPList
I think that similarly to #402
<#402>, we should make this an
object
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#399 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABc-aqaugBd4D8a3ZvQnUqbdhOy_khu0ks5uFwVrgaJpZM4VJlOa>
.
|
|
All good (with my nit-picky changes). I'm going to merge this into #395 myself now. |
fisx
force-pushed
the
spar-more-endpoints
branch
from
1b68688
to
d4ed906
Compare
|
thanks! |
|
(missing: idp-put endpoint; tests. i'll add that now to the parent PR.) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.