Added "connector_id" to skip straight to a connector (similar to when len(connector) is 1. #1481
Conversation
There was a problem hiding this comment.
Thank you for your contribution! 🎉
- Would you mind adding a test? And maybe a sentence somewhere explaining that this is a feature now?
- Do you think this should interact with the "show back link" switch? It was introduced with show "back" link for password connectors #1123, and depends on
len(connector)==1, too, if I remember correctly.
| @@ -233,6 +233,15 @@ func (s *Server) handleAuthorization(w http.ResponseWriter, r *http.Request) { | |||
| return | |||
| } | |||
|
|
|||
| if authReq.ConnectorID != "" { | |||
| for _, c := range connectors { | |||
| if c.ID == authReq.ConnectorID { | |||
There was a problem hiding this comment.
❓ Should this return an error if connector_id was specified but doesn't match any?
There was a problem hiding this comment.
I debated this myself. What could it ever do with an error other than break flow completely or just return to the index?
There was a problem hiding this comment.
That's true, but it's still better than 🙈ignoring it, isn't it? I'd propose calling s.tokenErrHelper, maybe?
There was a problem hiding this comment.
This would not change anything with respect to showBacklink (#1123), since the number of connectors doesn't actually change. It would need an additional test such as authReq.ConnectorID != "" to suppress the link.
There was a problem hiding this comment.
I'm not sure we'd actually want to suppress the link, would we? Maybe you'd like to use a different method...? 🤔
|
❔ Could you reset the commit's author to match your github user account? Just so the attribution works out well. |
|
@srenatus I'm looking for a place to document this, but I can't find any mention of the index page of connectors in the workflow. Docs like "using-dex.md" just say, "Dex determines user's identity". Any ideas? |
| if c.ID == connectorID { | ||
| break | ||
| } | ||
| return req, &authErr{"", "", errInvalidRequest, "Invalid ConnectorID"} |
There was a problem hiding this comment.
With this inside the for loop, it'll fail if there are more than one connectors, and the second one is the requested one. It would be nice if we had another test case for this.
There was a problem hiding this comment.
Added a validate function. Added a second connector to the test server. Added test.
There was a problem hiding this comment.
All right.So I ended up duplicating newTestServer to have newTestServerMultipleConnectors. It seems that the code workflow tests may depend on having a single connector, so adding in a second connector in order to test broke all of that.
There was a problem hiding this comment.
This now tests for selecting a connector other than the first.
| @@ -233,6 +233,15 @@ func (s *Server) handleAuthorization(w http.ResponseWriter, r *http.Request) { | |||
| return | |||
| } | |||
|
|
|||
| if authReq.ConnectorID != "" { | |||
| for _, c := range connectors { | |||
| if c.ID == authReq.ConnectorID { | |||
There was a problem hiding this comment.
I'm not sure we'd actually want to suppress the link, would we? Maybe you'd like to use a different method...? 🤔
Working code over documentation 😉 We could add a form field to example-app. I think that would be pretty neat. Let me know if you find this request just too annoying, and I'll pick up where you've left off. At any rate, let's prepare to merge this -- would you mind squashing your commits? 😃 |
LanceH
force-pushed
the
master
branch
2 times, most recently
from
548d22e
to
df8d006
Compare
|
Commits are squashed. |
| @@ -34,7 +34,7 @@ callback URL path: | |||
| ( dex issuer URL )/callback/( connector id )?( url query ) | |||
| ``` | |||
|
|
|||
| For example, if dex is running at `https://auth.example.com/dex` and the connector | |||
| For example, if dex is running at `https:/0/auth.example.com/dex` and the connector | |||
|
Travis looks stuck...? @LanceH could you rebase upon master and f-push? 🤞 maybe it'll be picked up then. |
|
Looks like Github had some outages, including this repo. I've kicked it off again. |
As a piece of "living documentation" for dexidp#1481. Signed-off-by: Stephan Renatus <[email protected]>
Added "connector_id" to skip straight to a connector (similar to when len(connector) is 1.
As a piece of "living documentation" for dexidp#1481. Signed-off-by: Stephan Renatus <[email protected]>
This would allow a request to bypass to a specific provider by providing connector_id.