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.

Sign up for GitHub

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

Jump to bottom

login_only #3728

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

login_only #3728

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c676272
login_only
johnwmail Apr 8, 2023
b3637f2
check the config flag first
johnwmail Apr 10, 2023
af86cdb
Merge branch 'iv-org:master' into master
johnwmail Jul 14, 2023
f524405
@SamantazFox: Put that code before return if (on line 64)
Jul 14, 2023
d3e6d7b
Set the cookie first
Jul 15, 2023
c3b06e7
Merge branch 'iv-org:master' into master
johnwmail Sep 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/invidious/config.cr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ class Config

# Used to tell Invidious it is behind a proxy, so links to resources should be https://
property https_only : Bool?
property login_only : Bool?
Copy link

Choose a reason for hiding this comment

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

It would be good to add a descriptive comment on what this does, similar to the others.

This comment was marked as outdated.

Sign in to view

# HMAC signing key for CSRF tokens and verifying pubsub subscriptions
property hmac_key : String = ""
# Domain to be used for links to resources on the site where an absolute URL is required
Expand Down
30 changes: 18 additions & 12 deletions src/invidious/routes/before_all.cr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,18 +61,6 @@ module Invidious::Routes::BeforeAll
env.response.headers["Strict-Transport-Security"] = "max-age=31536000; includeSubDomains; preload"
end

return if {
"/sb/",
"/vi/",
"/s_p/",
"/yts/",
"/ggpht/",
"/api/manifest/",
"/videoplayback",
"/latest_version",
"/download",
}.any? { |r| env.request.resource.starts_with? r }

if env.request.cookies.has_key? "SID"
sid = env.request.cookies["SID"].value

Expand Down Expand Up @@ -100,6 +88,24 @@ module Invidious::Routes::BeforeAll
end
end

unregistered_path_whitelist = {"/", "/login", "/licenses", "/privacy"}

This comment was marked as outdated.

Sign in to view

Copy link

Choose a reason for hiding this comment

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

I think /videoplayback needs to be added, otherwise I was not able to watch videos (DASH).

if CONFIG.login_only && !env.get?("user") && !unregistered_path_whitelist.includes?(env.request.path)
Comment on lines +91 to +92

This comment was marked as outdated.

Sign in to view

Copy link

Choose a reason for hiding this comment

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

This is good, but I also found that Clipious client requires /api/v1/stat when adding a new server to the client. It loads this to check it is a valid server, and refuses if it doesn't work (currently it returns a 500 error, which should be a bug anyway). However, even if this additional endpoint is added the client doesn't load Popular and Trending (default pane) because it makes these requests unauthenticated (will need to raise this as an issue with Clipious)

Copy link

Choose a reason for hiding this comment

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

Added your input to a new suggestion.

env.response.headers["Location"] = "/login"
haltf env, status_code: 302
Comment on lines +93 to +94

This comment was marked as outdated.

Sign in to view

end
Comment on lines +91 to +95

This comment was marked as outdated.

Sign in to view


return if {
"/sb/",
"/vi/",
"/s_p/",
"/yts/",
"/ggpht/",
"/api/manifest/",
"/videoplayback",
"/latest_version",
"/download",
}.any? { |r| env.request.resource.starts_with? r }

dark_mode = convert_theme(env.params.query["dark_mode"]?) || preferences.dark_mode.to_s
thin_mode = env.params.query["thin_mode"]? || preferences.thin_mode.to_s
thin_mode = thin_mode == "true"
Expand Down