SAML Authentication #1192
SAML Authentication #1192
Conversation
|
Coverage decreased (-0.06%) to 93.322% when pulling 5e917f7563674235f56933341aa499e1b7615974 on MarkusHarmsen:saml_authentication into 068c9fc on errbit:master. |
2 similar comments
|
Coverage decreased (-0.06%) to 93.322% when pulling 5e917f7563674235f56933341aa499e1b7615974 on MarkusHarmsen:saml_authentication into 068c9fc on errbit:master. |
|
Coverage decreased (-0.06%) to 93.322% when pulling 5e917f7563674235f56933341aa499e1b7615974 on MarkusHarmsen:saml_authentication into 068c9fc on errbit:master. |
|
@MarkusHarmsen this looks great. I've been putting this PR off too long and I think its time to focus on it because there's a lot of value here for SAML users. I don't particularly want the Vagrantfile, but still interested in why you have it. Are you just more familiar with Vagrant? or did you have a problem with the Docker setup? |
MarkusHarmsen
force-pushed
the
saml_authentication
branch
from
5e917f7
to
1318c23
Compare
|
Coverage decreased (-0.05%) to 93.156% when pulling 1318c2374fc1be389a3c935cfbf0c604f80b5200 on MarkusHarmsen:saml_authentication into 7d7c81f on errbit:master. |
1 similar comment
|
Coverage decreased (-0.05%) to 93.156% when pulling 1318c2374fc1be389a3c935cfbf0c604f80b5200 on MarkusHarmsen:saml_authentication into 7d7c81f on errbit:master. |
MarkusHarmsen
force-pushed
the
saml_authentication
branch
from
1318c23
to
fb245e5
Compare
MarkusHarmsen
force-pushed
the
saml_authentication
branch
from
fb245e5
to
6a8209b
Compare
1 similar comment
|
@stevecrozz I removed the Vagrant stuff and updated the pull request. The reason for Vagrant was just that I'm more familiar with Vagrant than docker ;) |
|
This is nice @MarkusHarmsen. One last question for you before I merge this. Are you willing to act as a sponsor for this feature, helping to answer questions and take occasional Errbit tickets relating to SAML auth? I ask because if we merge this, then we're also declaring that we support it. If our users start using this, they'll definitely raise issues and ask questions, and I don't always have the time to answer. |
|
Sure - before merging this, let me one time completely re-setup an errbit-instance an configure it using SAML (GSuite). I will document all required steps, so that might be a good starting point for a better documentation. |
|
Sounds great @MarkusHarmsen! |
|
@MarkusHarmsen it looks like there's another feature that will probably arrive in master soon that will conflict with this one: #1221. I don't think it will be very difficult to resolve, but it seems likely to happen. |
|
@MarkusHarmsen This PR is so good and so close, I'd hate to let it fade away. Are you still intending to add more documentation? |
| @@ -79,6 +81,25 @@ def google_oauth2 | |||
| end | |||
| end | |||
|
|
|||
| def saml | |||
| saml_uid = env['omniauth.auth'].uid | |||
| saml_user = User.where(email: saml_uid).first | |||
There was a problem hiding this comment.
Can we change it a little bit to use find_or_create method?
user = User.find_or_create!(email: saml_uid) do |record|
record.name = saml_uid
record.saml_uid = saml_uid
end
As discussed in #955, a SAML authentication could be useful for some users (at least it is for me).
Here is a proposal on how to implement SAML authentication. It has been tested with the GSuite.