Skip to content
Projects
Groups
Snippets
Help
Loading...
Help
Support
Keyboard shortcuts
?
Submit feedback
Contribute to GitLab
Sign in / Register
Toggle navigation
G
gitlab-ce
Project overview
Project overview
Details
Activity
Releases
Repository
Repository
Files
Commits
Branches
Tags
Contributors
Graph
Compare
Issues
0
Issues
0
List
Boards
Labels
Milestones
Merge Requests
0
Merge Requests
0
Analytics
Analytics
Repository
Value Stream
Wiki
Wiki
Snippets
Snippets
Members
Members
Collapse sidebar
Close sidebar
Activity
Graph
Create a new issue
Commits
Issue Boards
Open sidebar
Jérome Perrin
gitlab-ce
Commits
c2a48fd1
Commit
c2a48fd1
authored
Jun 25, 2018
by
Rémy Coutable
Browse files
Options
Browse Files
Download
Email Patches
Plain Diff
Ignore unknown OAuth sources in ApplicationSetting
Signed-off-by:
Rémy Coutable
<
remy@rymai.me
>
parent
701adc21
Changes
4
Hide whitespace changes
Inline
Side-by-side
Showing
4 changed files
with
60 additions
and
17 deletions
+60
-17
app/models/application_setting.rb
app/models/application_setting.rb
+5
-8
changelogs/unreleased/46783-removed-omniauth-provider-causing-invalid-application-setting.yml
...omniauth-provider-causing-invalid-application-setting.yml
+5
-0
spec/features/admin/admin_settings_spec.rb
spec/features/admin/admin_settings_spec.rb
+23
-0
spec/models/application_setting_spec.rb
spec/models/application_setting_spec.rb
+27
-9
No files found.
app/models/application_setting.rb
View file @
c2a48fd1
...
...
@@ -212,14 +212,6 @@ class ApplicationSetting < ActiveRecord::Base
end
end
validates_each
:disabled_oauth_sign_in_sources
do
|
record
,
attr
,
value
|
value
&
.
each
do
|
source
|
unless
Devise
.
omniauth_providers
.
include?
(
source
.
to_sym
)
record
.
errors
.
add
(
attr
,
"'
#{
source
}
' is not an OAuth sign-in source"
)
end
end
end
validate
:terms_exist
,
if: :enforce_terms?
before_validation
:ensure_uuid!
...
...
@@ -330,6 +322,11 @@ class ApplicationSetting < ActiveRecord::Base
::
Gitlab
::
Database
.
cached_column_exists?
(
:application_settings
,
:sidekiq_throttling_enabled
)
end
def
disabled_oauth_sign_in_sources
=
(
sources
)
sources
=
(
sources
||
[]).
map
(
&
:to_s
)
&
Devise
.
omniauth_providers
.
map
(
&
:to_s
)
super
(
sources
)
end
def
domain_whitelist_raw
self
.
domain_whitelist
&
.
join
(
"
\n
"
)
end
...
...
changelogs/unreleased/46783-removed-omniauth-provider-causing-invalid-application-setting.yml
0 → 100644
View file @
c2a48fd1
---
title
:
Ignore unknown OAuth sources in ApplicationSetting
merge_request
:
20129
author
:
type
:
fixed
spec/features/admin/admin_settings_spec.rb
View file @
c2a48fd1
...
...
@@ -124,6 +124,29 @@ feature 'Admin updates settings' do
expect
(
Gitlab
::
CurrentSettings
.
disabled_oauth_sign_in_sources
).
not_to
include
(
'google_oauth2'
)
end
scenario
'Oauth providers do not raise validation errors when saving unrelated changes'
do
expect
(
Gitlab
::
CurrentSettings
.
disabled_oauth_sign_in_sources
).
to
be_empty
page
.
within
(
'.as-signin'
)
do
uncheck
'Google'
click_button
'Save changes'
end
expect
(
page
).
to
have_content
"Application settings saved successfully"
expect
(
Gitlab
::
CurrentSettings
.
disabled_oauth_sign_in_sources
).
to
include
(
'google_oauth2'
)
# Remove google_oauth2 from the Omniauth strategies
allow
(
Devise
).
to
receive
(
:omniauth_providers
).
and_return
([])
# Save an unrelated setting
page
.
within
(
'.as-ci-cd'
)
do
click_button
'Save changes'
end
expect
(
page
).
to
have_content
"Application settings saved successfully"
expect
(
Gitlab
::
CurrentSettings
.
disabled_oauth_sign_in_sources
).
to
include
(
'google_oauth2'
)
end
scenario
'Change Help page'
do
page
.
within
(
'.as-help-page'
)
do
fill_in
'Help page text'
,
with:
'Example text'
...
...
spec/models/application_setting_spec.rb
View file @
c2a48fd1
...
...
@@ -25,15 +25,6 @@ describe ApplicationSetting do
it
{
is_expected
.
to
allow_value
(
https
).
for
(
:after_sign_out_path
)
}
it
{
is_expected
.
not_to
allow_value
(
ftp
).
for
(
:after_sign_out_path
)
}
describe
'disabled_oauth_sign_in_sources validations'
do
before
do
allow
(
Devise
).
to
receive
(
:omniauth_providers
).
and_return
([
:github
])
end
it
{
is_expected
.
to
allow_value
([
'github'
]).
for
(
:disabled_oauth_sign_in_sources
)
}
it
{
is_expected
.
not_to
allow_value
([
'test'
]).
for
(
:disabled_oauth_sign_in_sources
)
}
end
describe
'default_artifacts_expire_in'
do
it
'sets an error if it cannot parse'
do
setting
.
update
(
default_artifacts_expire_in:
'a'
)
...
...
@@ -314,6 +305,33 @@ describe ApplicationSetting do
end
end
describe
'#disabled_oauth_sign_in_sources='
do
before
do
allow
(
Devise
).
to
receive
(
:omniauth_providers
).
and_return
([
:github
])
end
it
'removes unknown sources (as strings) from the array'
do
subject
.
disabled_oauth_sign_in_sources
=
%w[github test]
expect
(
subject
).
to
be_valid
expect
(
subject
.
disabled_oauth_sign_in_sources
).
to
eq
[
'github'
]
end
it
'removes unknown sources (as symbols) from the array'
do
subject
.
disabled_oauth_sign_in_sources
=
%i[github test]
expect
(
subject
).
to
be_valid
expect
(
subject
.
disabled_oauth_sign_in_sources
).
to
eq
[
'github'
]
end
it
'ignores nil'
do
subject
.
disabled_oauth_sign_in_sources
=
nil
expect
(
subject
).
to
be_valid
expect
(
subject
.
disabled_oauth_sign_in_sources
).
to
be_empty
end
end
context
'restricted signup domains'
do
it
'sets single domain'
do
setting
.
domain_whitelist_raw
=
'example.com'
...
...
Write
Preview
Markdown
is supported
0%
Try again
or
attach a new file
Attach a file
Cancel
You are about to add
0
people
to the discussion. Proceed with caution.
Finish editing this message first!
Cancel
Please
register
or
sign in
to comment