Skip to content

Remove Access-Control-Allow-Origin#2484

Closed
jvoisin wants to merge 1 commit intoAdguardTeam:masterfrom
jvoisin:remove_allow_origin
Closed

Remove Access-Control-Allow-Origin#2484
jvoisin wants to merge 1 commit intoAdguardTeam:masterfrom
jvoisin:remove_allow_origin

Conversation

@jvoisin
Copy link
Contributor

@jvoisin jvoisin commented Dec 23, 2020

It doesn't server any purpose, and allows website to
probe if AdGuard Home is currently running on the LAN of the
user by bruteforcing common IP addresses (192.168.1.0/24 and 192.168.0.0/24)
until one of them returns AGH's html.

It doesn't server any purpose, and allows website to
probe if AdGuard Home is currently running on the LAN of the
user by bruteforcing common IP addresses (192.168.1.0/24 and 192.168.0.0/24)
until one of them returns AGH's html.
@codecov-io
Copy link

Codecov Report

Merging #2484 (b1c7ebd) into master (e829e7a) will increase coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2484   +/-   ##
=======================================
  Coverage   38.66%   38.66%           
=======================================
  Files          84       84           
  Lines        9471     9470    -1     
=======================================
  Hits         3662     3662           
+ Misses       5351     5350    -1     
  Partials      458      458           
Impacted Files Coverage Δ
internal/home/control.go 0.00% <ø> (ø)

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 e829e7a...b1c7ebd. Read the comment docs.

@ameshkov ameshkov requested a review from ainar-g December 28, 2020 10:19
@ameshkov
Copy link
Member

@ainar-g please check the commits history. I don't remember why we added, but I believe there was a reason for this.

@ainar-g ainar-g self-assigned this Feb 5, 2021
adguard pushed a commit that referenced this pull request Feb 5, 2021
Merge in DNS/adguard-home from 2484-access-control to master

Updates #2484.

Squashed commit of the following:

commit 4f0c6dd
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Fri Feb 5 12:42:22 2021 +0300

    home: don't allow all origins
@ainar-g
Copy link
Contributor

ainar-g commented Feb 5, 2021

Thanks for the contribution! I've analyzed the history of that line and discovered that we do need to set the header in some cases (the main one being the state after the user enables HTTPS redirect) but probably not to *. (And the code also required some refactoring.) See bc01f4f. I will close this PR, but feel free to reopen if you think that something else could be improved there.

@ainar-g ainar-g closed this Feb 5, 2021
heyxkhoa pushed a commit to heyxkhoa/AdGuardHome that referenced this pull request Mar 20, 2023
Merge in DNS/adguard-home from 2484-access-control to master

Updates AdguardTeam#2484.

Squashed commit of the following:

commit 4f0c6dd
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Fri Feb 5 12:42:22 2021 +0300

    home: don't allow all origins
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants