Skip to content

Commit f228617

Browse files
authored
Update single_sign_on.rb to avoid 500 errors on malformed sso queries (#258)
That string typically comes from the end user so it may not contain sso parameter at all
1 parent 92fac1a commit f228617

File tree

2 files changed

+26
-9
lines changed

2 files changed

+26
-9
lines changed

lib/discourse_api/single_sign_on.rb

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -98,10 +98,10 @@ def self.parse(payload, sso_secret = nil)
9898
sso.sso_secret = sso_secret if sso_secret
9999

100100
parsed = Rack::Utils.parse_query(payload)
101-
if sso.sign(parsed["sso"]) != parsed["sig"]
101+
if parsed["sso"].nil? || sso.sign(parsed["sso"]) != parsed["sig"]
102102
diags =
103-
"\n\nsso: #{parsed["sso"]}\n\nsig: #{parsed["sig"]}\n\nexpected sig: #{sso.sign(parsed["sso"])}"
104-
if parsed["sso"] =~ %r{[^a-zA-Z0-9=\r\n/+]}m
103+
"\n\nsso: #{parsed["sso"].inspect}\n\nsig: #{parsed["sig"].inspect}\n\nexpected sig: #{sso.sign(parsed.fetch("sso", ""))}"
104+
if parsed["sso"].nil? || parsed["sso"] =~ %r{[^a-zA-Z0-9=\r\n/+]}m
105105
raise ParseError,
106106
"The SSO field should be Base64 encoded, using only A-Z, a-z, 0-9, +, /, and = characters. Your input contains characters we don't understand as Base64, see http://en.wikipedia.org/wiki/Base64 #{diags}"
107107
else

spec/discourse_api/single_sign_on_spec.rb

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,12 +32,29 @@
3232
end
3333

3434
describe ".parse" do
35-
it "raises ParseError when there's a signature mismatch" do
36-
sso = described_class.new
37-
sso.sso_secret = "abcd"
38-
expect { described_class.parse(sso.payload, "dcba") }.to raise_error(
39-
DiscourseApi::SingleSignOn::ParseError,
40-
)
35+
context "when sso is present" do
36+
it "raises ParseError when there's a signature mismatch" do
37+
sso = described_class.new
38+
sso.sso_secret = "abcd"
39+
expect { described_class.parse(sso.payload, "dcba") }.to raise_error(
40+
DiscourseApi::SingleSignOn::ParseError,
41+
)
42+
end
43+
end
44+
45+
context "when sso is missing" do
46+
it "raises ParseError when there's a signature mismatch" do
47+
sso = described_class.new
48+
sso.sso_secret = "abcd"
49+
missing_sso = Rack::Utils.parse_query(sso.payload)
50+
missing_sso.delete("sso")
51+
malformed_query = Rack::Utils.build_query(missing_sso)
52+
53+
expect { described_class.parse(malformed_query, "dcba") }.to raise_error(
54+
DiscourseApi::SingleSignOn::ParseError,
55+
/The SSO field should/i,
56+
)
57+
end
4158
end
4259
end
4360
end

0 commit comments

Comments
 (0)