Skip to content

Populate avatar from SAML attribute #520

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
Lumrenion wants to merge 41 commits into from
Closed

Populate avatar from SAML attribute #520

Lumrenion wants to merge 41 commits into from

Conversation

@Lumrenion
Copy link

@Lumrenion Lumrenion commented Mar 23, 2021

This PR addresses #187.

  • In admin configuration, a new configuration field avatar_mapping is added.
  • If avatar_mapping configuration field is not empty, the user wont be able to upload an image in the nextcloud profile settings
  • The avatar_mapping field holds a publicly available URL to the profile image of the user. As usual, the image will be downloaded to nextcloud when the user logs in via user_saml.

@Lumrenion
Copy link
Author

Lumrenion commented May 15, 2021

It is my first code contribution to a Nextcloud project, so I am not sure, how the merging process works. Do I need to do something for this PR being merged?

blizzz
blizzz requested changes May 17, 2021
View reviewed changes
@blizzz blizzz requested a review from LukasReschke May 17, 2021 08:53
@Lumrenion Lumrenion requested a review from blizzz July 27, 2021 12:18
nextcloud-bot and others added 26 commits August 26, 2021 12:52
@nextcloud-bot
[tx-robot] updated from transifex
58c8706 
Signed-off-by: Nextcloud bot <bot@nextcloud.com>
Signed-off-by: Philipp Seßner <mail@philpp-sessner.de>
#187 Add avatar_mapping to admin settings
45a1b8e 
Signed-off-by: Philipp Seßner <mail@philpp-sessner.de>
#187 Add german translations for avatar_mapping in admin settings
258b77c 
Signed-off-by: Philipp Seßner <mail@philpp-sessner.de>
#187 Implement getting avatar from publicly available URL with avatar…
354a3ad 
…_mapping config setting and disabling image upload if avatar_mapping configuration is set

Signed-off-by: Philipp Seßner <mail@philpp-sessner.de>
#187 Adapt test for avatar_mapping in AdminTest
50c7ae6 
Signed-off-by: Philipp Seßner <mail@philpp-sessner.de>
Remove translation as they are managed in transifex
4e57d60 
Signed-off-by: Philipp Seßner <mail@philpp-sessner.de>
Change indentation from 4 spaces to tabs, according to coding guidelines
a967f77 
Signed-off-by: Philipp Seßner <mail@philpp-sessner.de>
Replace LDAP FilesystemHelper with native methods
7ef0529 
Signed-off-by: Philipp Seßner <mail@philpp-sessner.de>
Minor refactoring of if statement when checking if a user can change …
e03bdfe 
…their avatar

Signed-off-by: Philipp Seßner <mail@philpp-sessner.de>
Give method to set the avatar from SAML a more speaking name
c448062 
Signed-off-by: Philipp Seßner <mail@philpp-sessner.de>
@nextcloud-bot
[tx-robot] updated from transifex
1252b40 
Signed-off-by: Nextcloud bot <bot@nextcloud.com>
Signed-off-by: Philipp Seßner <mail@philpp-sessner.de>
@dependabot-preview
Bump guzzlehttp/guzzle from 7.0.1 to 7.3.0 in /tests/integration
e0546ef 
Bumps [guzzlehttp/guzzle](https://github.com/guzzle/guzzle) from 7.0.1 to 7.3.0.
- [Release notes](https://github.com/guzzle/guzzle/releases)
- [Changelog](https://github.com/guzzle/guzzle/blob/master/CHANGELOG.md)
- [Commits](guzzle/guzzle@7.0.1...7.3.0)

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>
Signed-off-by: Philipp Seßner <mail@philpp-sessner.de>
@nextcloud-bot
[tx-robot] updated from transifex
6caa617 
Signed-off-by: Nextcloud bot <bot@nextcloud.com>
Signed-off-by: Philipp Seßner <mail@philpp-sessner.de>
@nextcloud-bot
[tx-robot] updated from transifex
c7517bd 
Signed-off-by: Nextcloud bot <bot@nextcloud.com>
Signed-off-by: Philipp Seßner <mail@philpp-sessner.de>
@nextcloud-bot
[tx-robot] updated from transifex
fa3416d 
Signed-off-by: Nextcloud bot <bot@nextcloud.com>
Signed-off-by: Philipp Seßner <mail@philpp-sessner.de>
@nextcloud-bot
[tx-robot] updated from transifex
469e600 
Signed-off-by: Nextcloud bot <bot@nextcloud.com>
Signed-off-by: Philipp Seßner <mail@philpp-sessner.de>
@nextcloud-bot
[tx-robot] updated from transifex
21cffba 
Signed-off-by: Nextcloud bot <bot@nextcloud.com>
Signed-off-by: Philipp Seßner <mail@philpp-sessner.de>
@nextcloud-bot
[tx-robot] updated from transifex
a659c30 
Signed-off-by: Nextcloud bot <bot@nextcloud.com>
Signed-off-by: Philipp Seßner <mail@philpp-sessner.de>
@nextcloud-bot
[tx-robot] updated from transifex
4856954 
Signed-off-by: Nextcloud bot <bot@nextcloud.com>
Signed-off-by: Philipp Seßner <mail@philpp-sessner.de>
@nextcloud-bot
[tx-robot] updated from transifex
98482ae 
Signed-off-by: Nextcloud bot <bot@nextcloud.com>
Signed-off-by: Philipp Seßner <mail@philpp-sessner.de>
@nextcloud-bot
[tx-robot] updated from transifex
be23456 
Signed-off-by: Nextcloud bot <bot@nextcloud.com>
Signed-off-by: Philipp Seßner <mail@philpp-sessner.de>
@nextcloud-bot
[tx-robot] updated from transifex
5a60cef 
Signed-off-by: Nextcloud bot <bot@nextcloud.com>
Signed-off-by: Philipp Seßner <mail@philpp-sessner.de>
@LukasReschke
Allow setting of "retrieveParametersFromServer"
8ca57c8 
Some SAML servers require this type of decoding, otherwise the SLO request fails. Ideally the library would perform both verifications (SAML-Toolkits/php-saml#466), but it seems upstream doesn't want to perform this change.

Until we have considered a better solution for this, this adds a new checkbox that one can configure.

Ref #403

Signed-off-by: Lukas Reschke <lukas@statuscode.ch>
Signed-off-by: Philipp Seßner <mail@philpp-sessner.de>
@LukasReschke
Add new checkbox to AdminTest
8142653 
Signed-off-by: Lukas Reschke <lukas@statuscode.ch>
Signed-off-by: Philipp Seßner <mail@philpp-sessner.de>
@LukasReschke
Actually replace $retrieveParametersFromServer parameter
f1eb5dd 
Signed-off-by: Lukas Reschke <lukas@statuscode.ch>
Signed-off-by: Philipp Seßner <mail@philpp-sessner.de>
@nextcloud-bot
[tx-robot] updated from transifex
d069181 
Signed-off-by: Nextcloud bot <bot@nextcloud.com>
Signed-off-by: Philipp Seßner <mail@philpp-sessner.de>
nextcloud-bot and others added 10 commits August 26, 2021 12:52
@nextcloud-bot
[tx-robot] updated from transifex
6d96acb 
Signed-off-by: Nextcloud bot <bot@nextcloud.com>
Signed-off-by: Philipp Seßner <mail@philpp-sessner.de>
@nextcloud-bot
[tx-robot] updated from transifex
d2ba906 
Signed-off-by: Nextcloud bot <bot@nextcloud.com>
Signed-off-by: Philipp Seßner <mail@philpp-sessner.de>
@nextcloud-bot
[tx-robot] updated from transifex
c32ca47 
Signed-off-by: Nextcloud bot <bot@nextcloud.com>
Signed-off-by: Philipp Seßner <mail@philpp-sessner.de>
@nextcloud-bot
[tx-robot] updated from transifex
6253cfc 
Signed-off-by: Nextcloud bot <bot@nextcloud.com>
Signed-off-by: Philipp Seßner <mail@philpp-sessner.de>
@nextcloud-bot
[tx-robot] updated from transifex
c959239 
Signed-off-by: Nextcloud bot <bot@nextcloud.com>
Signed-off-by: Philipp Seßner <mail@philpp-sessner.de>
@rullzer
Bump version
8caf2d3 
Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
Signed-off-by: Philipp Seßner <mail@philpp-sessner.de>
Use dependency injection for avatar manager
102cd00 
Signed-off-by: Philipp Seßner <mail@philpp-sessner.de>
Only update avatar image if its checksum value has changed
ebb4413 
Signed-off-by: Philipp Seßner <mail@philpp-sessner.de>
merge changes from origin
e6a3369
#187 revert adding german translation as translations are done via tr…
fd4c9ae 
…ansifex

Signed-off-by: Philipp Seßner <mail@philpp-sessner.de>
Lumrenion
Lumrenion commented Aug 26, 2021
View reviewed changes
Copy link
Author

@Lumrenion Lumrenion left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think. all change requests in the last review are addressed now. I also fetched the updates from master, so the PR can be reviewed again.

@solracsf solracsf linked an issue Dec 3, 2021 that may be closed by this pull request
Populate avatar from SAML attribute #187
Open
@RailForWork
Copy link

RailForWork commented Jan 10, 2022

Love this function, when will it be commit?

@blizzz
Copy link
Member

blizzz commented Jan 11, 2022

I missed the notification in August. Will revisit after #558 is merged and shipped.

@RailForWork
Copy link

RailForWork commented Apr 15, 2022

I missed the notification in August. Will revisit after #558 is merged and shipped.

Cool, thanks for your working.

@blizzz
Copy link
Member

blizzz commented Jun 1, 2022

/rebase

@blizzz
Copy link
Member

blizzz commented Jun 1, 2022

@Lumrenion could you rebase, please?

@Lumrenion
Copy link
Author

Lumrenion commented Jul 6, 2022

I rebased the PR and did the necessary changes for this to work with the new SAMLSettings

@blizzz
Copy link
Member

blizzz commented Jul 8, 2022

Not sure the rebase worked with the 40 commits added here

@Lumrenion
Copy link
Author

Lumrenion commented Jul 9, 2022

True, it has commits on my fork that definitely came from this master. Should I create a new fork and manually do the changes again?

@blizzz
Copy link
Member

blizzz commented Jul 22, 2022

True, it has commits on my fork that definitely came from this master. Should I create a new fork and manually do the changes again?

either retry to rebase, or do a new PR. You can apply your commits, not need to redo everything by hand.

@Lumrenion
Copy link
Author

Lumrenion commented Aug 3, 2022

Created a new PR with the actual changes only: #640

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@LukasReschke LukasReschke Awaiting requested review from LukasReschke

@blizzz blizzz Awaiting requested review from blizzz

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Populate avatar from SAML attribute

6 participants

@Lumrenion @RailForWork @blizzz @nextcloud-bot @LukasReschke @rullzer