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

Allow regular users to change their CORS domains #30648

Merged
merged 1 commit into from
Feb 28, 2018

Conversation

PVince81
Copy link
Contributor

Description

See subject

Related Issue

Fixes #30647

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@codecov
Copy link

codecov bot commented Feb 28, 2018

Codecov Report

Merging #30648 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master   #30648   +/-   ##
=========================================
  Coverage      61.9%    61.9%           
  Complexity    19061    19061           
=========================================
  Files          1091     1091           
  Lines         61473    61473           
=========================================
  Hits          38052    38052           
  Misses        23421    23421
Impacted Files Coverage Δ Complexity Δ
settings/Controller/CorsController.php 97.5% <ø> (ø) 10 <0> (ø) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 64b847b...cac7438. Read the comment docs.

Copy link
Member

@DeepDiver1975 DeepDiver1975 left a comment

Choose a reason for hiding this comment

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

Tested - works 👍

@PVince81
Copy link
Contributor Author

stable10: #30649

@pako81
Copy link
Contributor

pako81 commented Feb 28, 2018

works. But should not the domains then appear in the list of the white-listed domains?

@pako81
Copy link
Contributor

pako81 commented Feb 28, 2018

ah ok, it looks like there is no error showed if you enter an invalid URL (i.e. missing the http part)

@PVince81
Copy link
Contributor Author

ah ok, it looks like there is no error showed if you enter an invalid URL (i.e. missing the http part)

yes, that's another bug... will take a bit more time to fix as there is no way to return an error here... need to port the code to use ajax calls instead of full page request

@PVince81 PVince81 merged commit 4c73a09 into master Feb 28, 2018
@PVince81 PVince81 deleted the settings-cors-regular-user branch February 28, 2018 15:41
@lock
Copy link

lock bot commented Jul 31, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Jul 31, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Users cannot add domains under CORS settings
4 participants