added support for saml login #1408
Conversation
kevinpapst
changed the title
|
Hi I get an error on successful saml response: bash-5.0$ tail -f var/log/prod.log Possible I am doing something wrong... |
|
Ah. It requires me to create the username first. It should default to creating the user like ldap. |
|
Accepted: |
|
not yet. home to today
…On Fri., Jan. 24, 2020, 7:21 a.m. Kevin Papst, ***@***.***> wrote:
Accepted: email is now the only required field, username will fallback to
email if not set.
Did you try groups already?
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#1408>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAH3N66FN46L247IVNKKEEDQ7LFM5ANCNFSM4KKZAKRQ>
.
|
|
Hi I successfully tested a login with a Microsoft Azure IDp however upon the second login it attempts to create a duplicate account. [2020-01-24 14:25:06] request.CRITICAL: Uncaught PHP Exception Doctrine\DBAL\Exception\UniqueConstraintViolationException: "An exception occurred while executing 'INSERT INTO kimai2_users (username, username_canonical, email, email_canonical, enabled, salt, password, last_login, confirmation_token, password_requested_at, roles, alias, registration_date, title, avatar, api_token) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)' with params ["tl@example.com", "tl@example.com", "tl@example.com", "tl@example.com", 1, null, "", null, null, null, "a:0:{}", null, "2020-01-24 14:25:06", null, null, null]: SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry 'tl@example.com' for key 'UNIQ_B9AC5BCE92FC23A8'" at /opt/kimai/vendor/doctrine/dbal/lib/Doctrine/DBAL/Driver/AbstractMySQLDriver.php line 55 {"exception":"[object] (Doctrine\DBAL\Exception\UniqueConstraintViolationException(code: 0): An exception occurred while executing 'INSERT INTO kimai2_users (username, username_canonical, email, email_canonical, enabled, salt, password, last_login, confirmation_token, password_requested_at, roles, alias, registration_date, title, avatar, api_token) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)' with params ["tl@example.com", "tl@example.com", "tl@example.com", "tl@example.com", 1, null, "", null, null, null, "a:0:{}", null, "2020-01-24 14:25:06", null, null, null]:\n\nSQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry 'tl@example.com' for key 'UNIQ_B9AC5BCE92FC23A8' at /opt/kimai/vendor/doctrine/dbal/lib/Doctrine/DBAL/Driver/AbstractMySQLDriver.php:55, Doctrine\DBAL\Driver\PDOException(code: 23000): SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry 'tl@example.com' for key 'UNIQ_B9AC5BCE92FC23A8' at /opt/kimai/vendor/doctrine/dbal/lib/Doctrine/DBAL/Driver/PDOStatement.php:123, PDOException(code: 23000): SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry 'tl@example.com' for key 'UNIQ_B9AC5BCE92FC23A8' at /opt/kimai/vendor/doctrine/dbal/lib/Doctrine/DBAL/Driver/PDOStatement.php:121)"} [] |
|
Maybe because of your previous tests? Check your Users are persisted and re-used if found in the database. |
|
No, there were only 3 users; My superadmin, google account, and the microsoft account I deleted the microsoft account and was able to log in. I closed the browser and tried again and I was not able to login |
|
Please add a and let me know what you see. Maybe the user has the wrong email? |
|
Sorted the duplicate I changed the local.yaml mapping from username: $Email to username: $http://schemas.xmlsoap.org/ws/2005/05/identity/claims/name How do the groups mapping work? An can I map the full name Full Name? |
|
I think you should be able to remove the username from the mapping completely, as it will default to that field - but I am not entirely sure. You can try to set the |
|
Working on the groups. I have Which as you may notice have spaces :-) when /opt/kimai/src/Auth/User/SamlUserFactory.php reads them ( var_dump($this->groupMapping); at line 52 I get: For some reason it is converting the - to an _ Any ideas? |
|
Oh please not 😬 okay right on, I will convert that "array like" syntax to the same that is used for LDAP. Now I can remember the reason ^^ |
|
@timlegge I updated the configuration syntax, you need to change the block in The attribute mapping from: to Then See https://www.kimai.org/documentation/saml.html Full example: |
|
testing for the last bit...
…On Fri., Jan. 24, 2020, 6:14 p.m. Kevin Papst, ***@***.***> wrote:
@timlegge <https://github.com/timlegge> I updated the configuration
syntax, you need to change the block in kimai.
The attribute mapping from:
mapping:
email: $Email
username: $Email
alias: $FullName
to
mapping:
- { saml: $Email, kimai: email }
- { saml: $Email, kimai: username }
- { saml: $FullName, kimai: alias }
Then groups was renamed to roles and the array syntax under roles was
also changed like the mapping above.
See https://www.kimai.org/documentation/saml.html
Full example:
kimai:
saml:
activate: true
title: Login with Google
mapping:
- { saml: $Email, kimai: email }
- { saml: $Email, kimai: username }
- { saml: $FullName, kimai: alias }
roles:
attribute: Roles
mapping:
- { saml: Admins, kimai: ROLE_ADMIN }
- { saml: Management, kimai: ROLE_TEAMLEAD }
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1408>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAH3N6ZQCFD6CCV2T3HK7JDQ7NR5XANCNFSM4KKZAKRQ>
.
|
|
@kevinpapst Initial testing looks promising. Nice! I have tested with one user (static group) against a Microsoft Azure Saml Identity Provider. With the change to the ldap style arrays my groups complete with spaces and dashes (I know, I know) works and the user is created. I intercepted the response from the server and tampered with the xml to attempt to grant extra permissions and as expected OneLogin caught the tampering with the signed Message - great I have not changed the group that the user is in yet but I did set the roles in the user record to ''. As I was logged in that changed my user to a normal user on page refresh I tried to log in again and the permissions were that of a normal user. It would be expected to refresh the user data on login. Especially the title, roles and possibly alias So far that is the only issue I have found. |
Things like that should be discussed upfront. The necessary change was really big, but it works now. When updating, you need to remove these two lines from the config (see updates docs): |
# Conflicts: # src/Voter/UserVoter.php
|
The role information is not updateing in the latest version. Looking for the issue now |
|
never mind. My issue - reading the documentation caused me an issue: roles: The attribute has to match the attribute name of the groups/roles being returned in the Saml Response |
|
Found one issue with running behind a nginx proxy: I will look at if thats a nginx resolution. [2020-01-29 16:06:40] request.CRITICAL: Uncaught PHP Exception RuntimeException: "The response was received at http://kimai-test.example.com:8001/auth/saml/acs instead of https://kimai-test.example.com/auth/saml/acs" at /opt/kimai/src/Saml/Controller/SamlController.php line 52 {"exception":"[object] (RuntimeException(code: 0): The response was received at http://kimai-test.example.com:8001/auth/saml/acs instead of https://kimai-test.example.com/auth/saml/acs at /opt/kimai/src/Saml/Controller/SamlController.php:52)"} [] |
|
My assumption is, that this is a general "problem" with the Symfony setup and not with the SAML module. Maybe you have to set the How can I test that on my end? Can you share a snippet of your proxy config, so I can setup one here? |
|
Ok, I did set it up and found a solution, which works at a first glance. Need more testing... Here is my nginx proxy config (nothing special I would say): This is my .env setting: And this is a saml specific part of my local.yaml (I think the |
|
Looking, my proxy is ... complex ... |
|
Yes, but it is not about the proxy, but about the two kimai configs (the forwarded for headers are pretty standard, aren't they?). Edit: I added a chapter to the documentation about proxies: https://www.kimai.org/documentation/webserver-configuration.html#reverse-proxy |
|
@timlegge I found one more piece of code, which might help ... probably we can even skip the usage of the baseurl with that change. see my latest push, especially this line: 04223e0#diff-918f42d3001f48647a833788daea0e66R23 |
|
Hi Its possible I have my proxies misconfigured but I still need the base enabled. Works fine with that enabled. Tim |
|
LOL, for the other customer the baseurl didn't work, but this setting was required. |
|
likely done tomorrow. |
|
I had my team test SAML auth for about 3 hours yesterday, and we cannot find any issues as of today. We did see an issue where the user would never be allowed to login if they first attempted to login using a non-SAML authenticated user:
We're now no longer able to reproduce that issue, but will keep an eye out for it nonetheless. It could have been resolved by browser cache and Kimai app cache. However, we did that prior to starting our testing. For the moment, I do agree with an earlier comment about the need to concatenate the 'first name' and 'last name' attribute. Google and idP do not provide 'Full Name' as an attribute. It is possible to create a 'Custom Attribute,' but for an organization with many users, this requires administrator access to do so, and can be time consuming. It would be incredibly powerful for us to be able to add the following: Or something similar. |
# Conflicts: # composer.json # composer.lock
|
usually when I see app notconfigured is when I accidentally use my gmail.com
account
…On Fri., Jan. 31, 2020, 11:34 a.m. urinal-cake, ***@***.***> wrote:
I had my team test SAML auth for about 3 hours yesterday, and we cannot
find any issues as of today.
We did see an issue where the user would never be allowed to login if they
first attempted to login using a non-SAML authenticated user:
1. Click Login button
2. Select Gmail account, or G Suite account for a different
organization other than target.
3. Notice *app_not_configured_for_user* error
4. Go back in browser
5. Utilize G Suite account for target organization
6. Notice same error as #3
<#3>, even though the
account is now valid.
We're now no longer able to reproduce that issue, but will keep an eye out
for it nonetheless. It could have been resolved by browser cache and Kimai
app cache. However, we did that prior to starting our testing.
For the moment, I do agree with an earlier comment about the need to
concatenate the 'first name' and 'last name' attribute. Google and idP do
not provide 'Full Name' as an attribute. It is possible to create a 'Custom
Attribute,' but for an organization with many users, this requires
administrator access to do so, and can be time consuming.
It would be *incredibly* powerful for us to be able to add the following:
mapping: - { saml: $Email, kimai: email } - { saml: $Email, kimai:
username } - { saml: $FirstName ~ $LastName, kimai: alias }
Or something similar.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1408>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAH3N6Y2ELDOP7EENH46RHLRARAKFANCNFSM4KKZAKRQ>
.
|
|
At that point in time kimai did not register the user, as no information about him is available - so I guess this problem won't reoccur. I added the concatenation of attributes, configure it like this: Its pure string replacement magic and each attribute in the Please test if it works and let me know. |
Codecov Report
@@ Coverage Diff @@
## master #1408 +/- ##
============================================
- Coverage 92.58% 92.57% -0.01%
- Complexity 4161 4169 +8
============================================
Files 394 396 +2
Lines 12799 12878 +79
============================================
+ Hits 11850 11922 +72
- Misses 949 956 +7
|
|
Kevin, this worked flawlessly. We did some more testing with the combined fullname attribute, but haven't turned up any more issues. Good to go from our end. Let me know how else I can help. |
|
@timlegge done as well or do you want some more testing time? |
|
@kevinpapst have not pulled the latest commit. |
|
Thanks @timlegge and @urinal-cake for all the testing!!!! Changes can still be done, when we find any issue. |
|
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. If you use Kimai on a daily basis, please consider donating to support further development of Kimai. |
Description
Only tested with Google apps SAML implementation
In order to test this branch, do a checkout and then:
config/packages/local.yamlwith the adjusted contents belowbin/console cache:clear --env=prodbin/console cache:warmup --env=prodYOU FIND THE REQUIRED CONFIGURATION IN THE DOCUMENTATION
See https://www.kimai.org/documentation/saml.html
Fixes #1227
Links:
Types of changes
Checklist
composer kimai:code-check)