Skip to content
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.

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

Populate avatar from SAML attribute #520

Closed
wants to merge 41 commits into from
Closed

Populate avatar from SAML attribute #520

wants to merge 41 commits into from

Conversation

Lumrenion
Copy link

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

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 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
Signed-off-by: Nextcloud bot <[email protected]>
Signed-off-by: Philipp Seßner <[email protected]>
…_mapping config setting and disabling image upload if avatar_mapping configuration is set

Signed-off-by: Philipp Seßner <[email protected]>
Signed-off-by: Nextcloud bot <[email protected]>
Signed-off-by: Philipp Seßner <[email protected]>
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] <[email protected]>
Signed-off-by: Philipp Seßner <[email protected]>
Signed-off-by: Nextcloud bot <[email protected]>
Signed-off-by: Philipp Seßner <[email protected]>
Signed-off-by: Nextcloud bot <[email protected]>
Signed-off-by: Philipp Seßner <[email protected]>
Signed-off-by: Nextcloud bot <[email protected]>
Signed-off-by: Philipp Seßner <[email protected]>
Signed-off-by: Nextcloud bot <[email protected]>
Signed-off-by: Philipp Seßner <[email protected]>
Signed-off-by: Nextcloud bot <[email protected]>
Signed-off-by: Philipp Seßner <[email protected]>
Signed-off-by: Nextcloud bot <[email protected]>
Signed-off-by: Philipp Seßner <[email protected]>
Signed-off-by: Nextcloud bot <[email protected]>
Signed-off-by: Philipp Seßner <[email protected]>
Signed-off-by: Nextcloud bot <[email protected]>
Signed-off-by: Philipp Seßner <[email protected]>
Signed-off-by: Nextcloud bot <[email protected]>
Signed-off-by: Philipp Seßner <[email protected]>
Signed-off-by: Nextcloud bot <[email protected]>
Signed-off-by: Philipp Seßner <[email protected]>
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 <[email protected]>
Signed-off-by: Philipp Seßner <[email protected]>
Signed-off-by: Lukas Reschke <[email protected]>
Signed-off-by: Philipp Seßner <[email protected]>
Signed-off-by: Lukas Reschke <[email protected]>
Signed-off-by: Philipp Seßner <[email protected]>
Signed-off-by: Nextcloud bot <[email protected]>
Signed-off-by: Philipp Seßner <[email protected]>
nextcloud-bot and others added 10 commits August 26, 2021 12:52
Signed-off-by: Nextcloud bot <[email protected]>
Signed-off-by: Philipp Seßner <[email protected]>
Signed-off-by: Nextcloud bot <[email protected]>
Signed-off-by: Philipp Seßner <[email protected]>
Signed-off-by: Nextcloud bot <[email protected]>
Signed-off-by: Philipp Seßner <[email protected]>
Signed-off-by: Nextcloud bot <[email protected]>
Signed-off-by: Philipp Seßner <[email protected]>
Signed-off-by: Nextcloud bot <[email protected]>
Signed-off-by: Philipp Seßner <[email protected]>
Signed-off-by: Roeland Jago Douma <[email protected]>
Signed-off-by: Philipp Seßner <[email protected]>
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
@RailForWork
Copy link

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

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

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

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

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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Populate avatar from SAML attribute
6 participants