Skip to content

Allow customizing Oauth providers for each tenant#5516

Merged
tramuntanal merged 3 commits intodecidim:masterfrom
PopulateTools:10-omniauth-multitenant
Jan 22, 2020
Merged

Allow customizing Oauth providers for each tenant#5516
tramuntanal merged 3 commits intodecidim:masterfrom
PopulateTools:10-omniauth-multitenant

Conversation

@amiedes
Copy link
Copy Markdown

@amiedes amiedes commented Nov 25, 2019 •
edited
Loading

🎩 What? Why?

This PR allows customizing the omniauth settings for each tenant, overriding the settings present in secrets.yml in case those were defined also.

📌 Related Issues

📋 Subtasks

  • Add CHANGELOG entry
  • Add documentation regarding the feature
  • Add tests

📷 Screenshots (optional)

image

@amiedes amiedes mentioned this pull request Nov 25, 2019
Closed
4 tasks
@amiedes amiedes marked this pull request as ready for review December 16, 2019 07:42
@amiedes
Copy link
Copy Markdown
Author

amiedes commented Dec 19, 2019

Whoops, this is ready for review, I forgot to request it.

cc @decidim/product

agustibr
agustibr reviewed Dec 24, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@agustibr agustibr left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Great work!
Is there a reason the ominauth providers' settings are not set on the new action?

They are only available on the organization#edit

paulinebessoles
paulinebessoles previously approved these changes Dec 27, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@paulinebessoles paulinebessoles left a comment

Choose a reason for hiding this comment

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

It's ok for us too !

@amiedes
Copy link
Copy Markdown
Author

amiedes commented Jan 2, 2020

Thanks for the review @agustibr @paulinebessoles!

Is there a reason the ominauth providers' settings are not set on the new action?

My bad, I'll add this asap.

@virgile-dev
Copy link
Copy Markdown
Contributor

virgile-dev commented Jan 10, 2020

@carolromero @tramuntanal any chance we can merge this into master soon we are waiting for the merge to backport into our clients version.

@carolromero
Copy link
Copy Markdown
Member

carolromero commented Jan 10, 2020

Hey @virgile-dev it is ok from @decidim/product!

tramuntanal
tramuntanal suggested changes Jan 15, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@tramuntanal tramuntanal left a comment

Choose a reason for hiding this comment

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

Great work @amiedes !
Just a couple of things:

  • ominauth providers' settings are still not set on the new action, can you add them please?
  • After releasing v0.20.0 the CHANGELOG.md should be manually rebuild from master.

@tramuntanal
Copy link
Copy Markdown
Contributor

tramuntanal commented Jan 17, 2020

Any news @amiedes ?

@amiedes
Copy link
Copy Markdown
Author

amiedes commented Jan 17, 2020

@tramuntanal Sorry for the delay, I expect to implement the requested changes on monday

@amiedes
Copy link
Copy Markdown
Author

amiedes commented Jan 21, 2020

@virgile-dev @tramuntanal changes adressed!

I believe the failing test is a flacky one, since in local it passes and I haven't touched that module.

@tramuntanal
Copy link
Copy Markdown
Contributor

tramuntanal commented Jan 21, 2020 •
edited
Loading

@amiedes yes, it probably is, can you re-run it?
Open the module ci/circleci details, and choose "Rerun job with SSH". I can not do it since the code is in your fork.

@amiedes
Copy link
Copy Markdown
Author

amiedes commented Jan 22, 2020

@tramuntanal green now!

@tramuntanal tramuntanal merged commit 012b5b1 into decidim:master Jan 22, 2020
@tramuntanal
Copy link
Copy Markdown
Contributor

tramuntanal commented Jan 22, 2020

Inn!! 👏

@microstudi microstudi mentioned this pull request Feb 18, 2020
Closed
@tramuntanal tramuntanal mentioned this pull request Feb 19, 2020
Merged
5 tasks
@microstudi microstudi mentioned this pull request Apr 28, 2020
Merged
10 tasks
@armandfardeau armandfardeau mentioned this pull request May 29, 2020
Closed
armandfardeau added a commit to OpenSourcePolitics/decidim that referenced this pull request Jun 2, 2020
@armandfardeau
Revert "Allow customizing Oauth providers for each tenant (decidim#5516…
e5fa576 
…)"

This reverts commit 012b5b1.
armandfardeau added a commit to OpenSourcePolitics/decidim that referenced this pull request Jun 9, 2020
@armandfardeau
Revert "Allow customizing Oauth providers for each tenant (decidim#5516…
b39d0f5 
…)" (#975)

This reverts commit 012b5b1.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@paulinebessoles paulinebessoles paulinebessoles left review comments

+2 more reviewers

@agustibr agustibr agustibr left review comments

@tramuntanal tramuntanal tramuntanal approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

in-review module: core

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@amiedes @virgile-dev @carolromero @tramuntanal @agustibr @paulinebessoles