-
Notifications
You must be signed in to change notification settings - Fork 167
sync_sso() adds its own "custom." prefix. This results in "custom.custom.field" in the payload #190
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
Changes from all commits
72512a6
d20eef8
829cc1e
4af8407
0255890
cca200e
20b641e
62923c6
70abd7a
60a8e9a
bcc8244
fee163a
49187be
8137837
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,6 +15,7 @@ class SingleSignOn | |
| #NONCE_EXPIRY_TIME = 10.minutes # minutes is a rails method and is causing an error. Is this needed in the api? | ||
|
|
||
| attr_accessor(*ACCESSORS) | ||
| attr_accessor :custom_fields | ||
| attr_writer :sso_secret, :sso_url | ||
|
|
||
| def self.sso_secret | ||
|
|
@@ -25,6 +26,36 @@ def self.sso_url | |
| raise RuntimeError, "sso_url not implemented on class, be sure to set it on instance" | ||
| end | ||
|
|
||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Initialization is similar to |
||
| def self.parse_hash(payload) | ||
| payload | ||
| sso = new | ||
|
|
||
| sso.sso_secret = payload.delete(:sso_secret) | ||
| sso.sso_url = payload.delete(:sso_url) | ||
|
|
||
| ACCESSORS.each do |k| | ||
| val = payload[k] | ||
|
|
||
| val = val.to_i if FIXNUMS.include? k | ||
| if BOOLS.include? k | ||
| val = ["true", "false"].include?(val) ? val == "true" : nil | ||
| end | ||
| val = Array(val) if ARRAYS.include?(k) && !val.nil? | ||
| sso.send("#{k}=", val) | ||
| end | ||
| # Set custom_fields | ||
| sso.custom_fields = payload[:custom_fields] | ||
| # Add custom_fields (old format) | ||
| payload.each do |k, v| | ||
| if field = k[/^custom\.(.+)$/, 1] | ||
| # Maintain adding of .custom bug | ||
| sso.custom_fields["custom.#{field}"] = v | ||
| end | ||
| end | ||
|
|
||
| sso | ||
| end | ||
|
|
||
| def self.parse(payload, sso_secret = nil) | ||
| sso = new | ||
| sso.sso_secret = sso_secret if sso_secret | ||
|
|
@@ -107,7 +138,5 @@ def unsigned_payload | |
|
|
||
| Rack::Utils.build_query(payload) | ||
| end | ||
|
|
||
| end | ||
|
|
||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,9 +4,51 @@ | |
| describe DiscourseApi::API::SSO do | ||
| subject { DiscourseApi::Client.new("#{host}", "test_d7fd0429940", "test_user") } | ||
|
|
||
| let(:params) do | ||
| { | ||
| sso_secret: 'abc', | ||
| sso_url: 'www.google.com', | ||
| name: 'Some User', | ||
| username: 'some_user', | ||
| email: 'some@email.com', | ||
| external_id: 'abc', | ||
| suppress_welcome_message: false, | ||
| avatar_url: 'https://www.website.com', | ||
| title: 'ruby', | ||
| avatar_force_update: false, | ||
| add_groups: ['a', 'b'], | ||
| remove_groups: ['c', 'd'], | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. old + new |
||
| # old format (which results in custom.custom.field_1 in unsigned_payload) | ||
| 'custom.field_1' => 'tomato', | ||
| # new format | ||
| custom_fields: { | ||
| field_2: 'potato' | ||
| } | ||
| } | ||
| end | ||
| let(:expected_unsigned_payload) do | ||
| 'name=Some+User&username=some_user&email=some%40email.com&'\ | ||
| 'avatar_url=https%3A%2F%2Fwww.website.com&external_id=abc&title=ruby'\ | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. note we maintain the custom.custom bug for existing users, new implementations should include |
||
| '&add_groups=a&add_groups=b&remove_groups=c&remove_groups=d&custom.field_2=potato&'\ | ||
| 'custom.custom.field_1=tomato' | ||
| end | ||
| let(:sso_double) { DiscourseApi::SingleSignOn.parse_hash(params) } | ||
|
|
||
| describe "#sync_sso" do | ||
| before do | ||
| stub_post(/.*sync_sso.*/).to_return(body: fixture("user.json"), headers: { content_type: "application/json" }) | ||
| stub_post(/.*sync_sso.*/).to_return( | ||
| body: fixture("user.json"), | ||
| headers: { content_type: "application/json" } | ||
| ) | ||
| end | ||
|
|
||
| it 'assigns params to sso instance' do | ||
| allow(DiscourseApi::SingleSignOn).to(receive(:parse_hash).with(params).and_return(sso_double)) | ||
|
|
||
| subject.sync_sso(params) | ||
|
|
||
| expect(sso_double.custom_fields).to eql({ 'custom.field_1' => 'tomato', :field_2 => 'potato' }) | ||
| expect(sso_double.unsigned_payload).to eql(expected_unsigned_payload) | ||
| end | ||
|
|
||
| it "requests the correct resource" do | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Separate to not break
ACCESSORS.eachloops