{"action":"created","issue":{"url":"https://api.github.com/repos/portier/portier-broker/issues/66","repository_url":"https://api.github.com/repos/portier/portier-broker","labels_url":"https://api.github.com/repos/portier/portier-broker/issues/66/labels{/name}","comments_url":"https://api.github.com/repos/portier/portier-broker/issues/66/comments","events_url":"https://api.github.com/repos/portier/portier-broker/issues/66/events","html_url":"https://github.com/portier/portier-broker/issues/66","id":185507219,"number":66,"title":"Refactor to provider selection to be more modular","user":{"login":"callahad","id":24193,"avatar_url":"https://avatars.githubusercontent.com/u/24193?v=3","gravatar_id":"","url":"https://api.github.com/users/callahad","html_url":"https://github.com/callahad","followers_url":"https://api.github.com/users/callahad/followers","following_url":"https://api.github.com/users/callahad/following{/other_user}","gists_url":"https://api.github.com/users/callahad/gists{/gist_id}","starred_url":"https://api.github.com/users/callahad/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/callahad/subscriptions","organizations_url":"https://api.github.com/users/callahad/orgs","repos_url":"https://api.github.com/users/callahad/repos","events_url":"https://api.github.com/users/callahad/events{/privacy}","received_events_url":"https://api.github.com/users/callahad/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":0,"created_at":"2016-10-26T20:53:20Z","updated_at":"2016-10-26T21:04:13Z","closed_at":null,"body":"Our level of abstraction doesn't feel right yet. Our [current code](https://github.com/portier/portier-broker/blob/a88f503702e085c868d5aaaa13598ae1f6ba3988/src/lib.rs#L318-L335) basically functions as:\r\n\r\n1. If `email.domain` in `providers`:\r\n    1. Get a redirect URL from the openid module\r\n    2. Redirect the user\r\n2. Else:\r\n    1. Get a session ID from the email module\r\n    2. Display the confirm email template to the user\r\n\r\nI don't think this is the best solution in a few areas:\r\n\r\n- It only switches on email domain. As per #44, we might want to switch on other attributes in the future.\r\n- If two domains are serviced by the same provider, as is the case with `gmail.com` and `googlemail.com` we have to duplicate that in the configuration.\r\n- General OIDC and Gmail-specific rules are commingled in `src/oidc.rs`\r\n- This design presumes that all providers are OIDC providers.\r\n- The returned values aren't unified: you get either a URL or a session ID.\r\n- The redirect logic is split between the individual module and `src/lib.rs`.\r\n\r\nI'm not sure how to best address this, but I think the answer might be along the lines of:\r\n\r\n- All providers adhere to a common interface or implement a common trait\r\n- Providers completely handle incoming requests and return a ready-to-use `iron::Response`\r\n- Split the generic OIDC provider and the Gmail provider into separate modules. The Gmail one can, of course, rely on facilities offered by the generic one.\r\n- Providers can return whether or not they can handle an address, either through a function returning `bool`, or by returning an `Option<Iron::Response>`.\r\n- We keep a prioritized list of providers, and just iterate over it, trying each one until one succeeds."},"comment":{"url":"https://api.github.com/repos/portier/portier-broker/issues/comments/256476426","html_url":"https://github.com/portier/portier-broker/issues/66#issuecomment-256476426","issue_url":"https://api.github.com/repos/portier/portier-broker/issues/66","id":256476426,"user":{"login":"stephank","id":89950,"avatar_url":"https://avatars.githubusercontent.com/u/89950?v=3","gravatar_id":"","url":"https://api.github.com/users/stephank","html_url":"https://github.com/stephank","followers_url":"https://api.github.com/users/stephank/followers","following_url":"https://api.github.com/users/stephank/following{/other_user}","gists_url":"https://api.github.com/users/stephank/gists{/gist_id}","starred_url":"https://api.github.com/users/stephank/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/stephank/subscriptions","organizations_url":"https://api.github.com/users/stephank/orgs","repos_url":"https://api.github.com/users/stephank/repos","events_url":"https://api.github.com/users/stephank/events{/privacy}","received_events_url":"https://api.github.com/users/stephank/received_events","type":"User","site_admin":false},"created_at":"2016-10-26T21:04:13Z","updated_at":"2016-10-26T21:04:13Z","body":"To be honest, this sounds a bit like over engineering the problem. There's no abstraction there right now, and maybe that's fine for what we're doing. We haven't even hashed out the details of native discovery yet."}}
{"actor":{"display_login":"stephank"}}