From 1a67fc6e991efedfbce8f6beaf4095ee2d946b3a Mon Sep 17 00:00:00 2001 From: Anbraten Date: Fri, 20 Aug 2021 16:32:52 +0200 Subject: [PATCH] Use server-host as source for public links and warn if it is set to localhost (#251) * Use server-host as source for public links * use config and rm GetURL() * fix: solve import cycle Co-authored-by: 6543 <6543@obermui.de> --- cmd/drone-server/server.go | 8 ++++- remote/bitbucket/bitbucket.go | 5 ++- remote/coding/coding.go | 6 ++-- remote/gitea/gitea_oauth.go | 5 ++- remote/github/github.go | 6 ++-- remote/gitlab/gitlab.go | 6 ++-- remote/gitlab3/gitlab.go | 6 ++-- server/build.go | 5 ++- server/configFetcher.go | 13 ++++++-- server/configFetcher_test.go | 15 ++++----- server/hook.go | 3 +- server/repo.go | 9 +++--- shared/httputil/httputil.go | 59 ----------------------------------- 13 files changed, 48 insertions(+), 98 deletions(-) diff --git a/cmd/drone-server/server.go b/cmd/drone-server/server.go index 8c909e059..6f669694d 100644 --- a/cmd/drone-server/server.go +++ b/cmd/drone-server/server.go @@ -69,6 +69,12 @@ func server(c *cli.Context) error { ) } + if strings.Contains(c.String("server-host"), "://localhost") { + logrus.Warningln( + "DRONE_HOST/DRONE_SERVER_HOST/WOODPECKER_HOST/WOODPECKER_SERVER_HOST should probably be publicly accessible (not localhost)", + ) + } + if strings.HasSuffix(c.String("server-host"), "/") { logrus.Fatalln( "DRONE_HOST/DRONE_SERVER_HOST/WOODPECKER_HOST/WOODPECKER_SERVER_HOST must not have trailing slash", @@ -224,7 +230,7 @@ func setupEvilGlobals(c *cli.Context, v store.Store, r remote.Remote) { droneserver.Config.Server.Cert = c.String("server-cert") droneserver.Config.Server.Key = c.String("server-key") droneserver.Config.Server.Pass = c.String("agent-secret") - droneserver.Config.Server.Host = strings.TrimRight(c.String("server-host"), "/") + droneserver.Config.Server.Host = c.String("server-host") droneserver.Config.Server.Port = c.String("server-addr") droneserver.Config.Server.RepoConfig = c.String("repo-config") droneserver.Config.Server.SessionExpires = c.Duration("session-expires") diff --git a/remote/bitbucket/bitbucket.go b/remote/bitbucket/bitbucket.go index 7d23c69a3..ce5f54a9f 100644 --- a/remote/bitbucket/bitbucket.go +++ b/remote/bitbucket/bitbucket.go @@ -22,7 +22,7 @@ import ( "github.com/woodpecker-ci/woodpecker/model" "github.com/woodpecker-ci/woodpecker/remote" "github.com/woodpecker-ci/woodpecker/remote/bitbucket/internal" - "github.com/woodpecker-ci/woodpecker/shared/httputil" + "github.com/woodpecker-ci/woodpecker/server" "golang.org/x/oauth2" ) @@ -54,8 +54,7 @@ func New(client, secret string) remote.Remote { // Login authenticates an account with Bitbucket using the oauth2 protocol. The // Bitbucket account details are returned when the user is successfully authenticated. func (c *config) Login(w http.ResponseWriter, req *http.Request) (*model.User, error) { - redirect := httputil.GetURL(req) - config := c.newConfig(redirect) + config := c.newConfig(server.Config.Server.Host) // get the OAuth errors if err := req.FormValue("error"); err != "" { diff --git a/remote/coding/coding.go b/remote/coding/coding.go index 2848c0179..f1715f56e 100644 --- a/remote/coding/coding.go +++ b/remote/coding/coding.go @@ -23,7 +23,7 @@ import ( "github.com/woodpecker-ci/woodpecker/model" "github.com/woodpecker-ci/woodpecker/remote" "github.com/woodpecker-ci/woodpecker/remote/coding/internal" - "github.com/woodpecker-ci/woodpecker/shared/httputil" + "github.com/woodpecker-ci/woodpecker/server" "golang.org/x/net/context" "golang.org/x/oauth2" @@ -62,8 +62,6 @@ func New(opts Opts) (remote.Remote, error) { remote.URL = strings.TrimSuffix(opts.URL, "/") } - // Hack to enable oauth2 access in coding's implementation - oauth2.RegisterBrokenAuthHeaderProvider(remote.URL) return remote, nil } @@ -81,7 +79,7 @@ type Coding struct { // Login authenticates the session and returns the // remote user details. func (c *Coding) Login(res http.ResponseWriter, req *http.Request) (*model.User, error) { - config := c.newConfig(httputil.GetURL(req)) + config := c.newConfig(server.Config.Server.Host) // get the OAuth errors if err := req.FormValue("error"); err != "" { diff --git a/remote/gitea/gitea_oauth.go b/remote/gitea/gitea_oauth.go index 5fd97676b..9595f5093 100644 --- a/remote/gitea/gitea_oauth.go +++ b/remote/gitea/gitea_oauth.go @@ -29,7 +29,7 @@ import ( "code.gitea.io/sdk/gitea" "github.com/woodpecker-ci/woodpecker/model" "github.com/woodpecker-ci/woodpecker/remote" - "github.com/woodpecker-ci/woodpecker/shared/httputil" + "github.com/woodpecker-ci/woodpecker/server" "golang.org/x/oauth2" ) @@ -78,7 +78,6 @@ func NewOauth(opts Opts) (remote.Remote, error) { // Login authenticates an account with Gitea using basic authentication. The // Gitea account details are returned when the user is successfully authenticated. func (c *oauthclient) Login(w http.ResponseWriter, req *http.Request) (*model.User, error) { - redirect := httputil.GetURL(req) config := &oauth2.Config{ ClientID: c.Client, ClientSecret: c.Secret, @@ -86,7 +85,7 @@ func (c *oauthclient) Login(w http.ResponseWriter, req *http.Request) (*model.Us AuthURL: fmt.Sprintf(authorizeTokenURL, c.URL), TokenURL: fmt.Sprintf(accessTokenURL, c.URL), }, - RedirectURL: fmt.Sprintf("%s/authorize", redirect), + RedirectURL: fmt.Sprintf("%s/authorize", server.Config.Server.Host), } // get the OAuth errors diff --git a/remote/github/github.go b/remote/github/github.go index 100ae562a..65272aea5 100644 --- a/remote/github/github.go +++ b/remote/github/github.go @@ -26,7 +26,7 @@ import ( "github.com/woodpecker-ci/woodpecker/model" "github.com/woodpecker-ci/woodpecker/remote" - "github.com/woodpecker-ci/woodpecker/shared/httputil" + "github.com/woodpecker-ci/woodpecker/server" "github.com/google/go-github/github" "golang.org/x/net/context" @@ -340,9 +340,9 @@ func (c *client) newConfig(req *http.Request) *oauth2.Config { intendedURL := req.URL.Query()["url"] if len(intendedURL) > 0 { - redirect = fmt.Sprintf("%s/authorize?url=%s", httputil.GetURL(req), intendedURL[0]) + redirect = fmt.Sprintf("%s/authorize?url=%s", server.Config.Server.Host, intendedURL[0]) } else { - redirect = fmt.Sprintf("%s/authorize", httputil.GetURL(req)) + redirect = fmt.Sprintf("%s/authorize", server.Config.Server.Host) } return &oauth2.Config{ diff --git a/remote/gitlab/gitlab.go b/remote/gitlab/gitlab.go index c401cc23e..1e713c028 100644 --- a/remote/gitlab/gitlab.go +++ b/remote/gitlab/gitlab.go @@ -26,7 +26,7 @@ import ( "github.com/woodpecker-ci/woodpecker/model" "github.com/woodpecker-ci/woodpecker/remote" - "github.com/woodpecker-ci/woodpecker/shared/httputil" + "github.com/woodpecker-ci/woodpecker/server" "github.com/woodpecker-ci/woodpecker/shared/oauth2" "github.com/woodpecker-ci/woodpecker/remote/gitlab/client" @@ -121,7 +121,7 @@ func (g *Gitlab) Login(res http.ResponseWriter, req *http.Request) (*model.User, Scope: DefaultScope, AuthURL: fmt.Sprintf("%s/oauth/authorize", g.URL), TokenURL: fmt.Sprintf("%s/oauth/token", g.URL), - RedirectURL: fmt.Sprintf("%s/authorize", httputil.GetURL(req)), + RedirectURL: fmt.Sprintf("%s/authorize", server.Config.Server.Host), } trans_ := &http.Transport{ @@ -631,7 +631,7 @@ func (g *Gitlab) Oauth2Transport(r *http.Request) *oauth2.Transport { Scope: DefaultScope, AuthURL: fmt.Sprintf("%s/oauth/authorize", g.URL), TokenURL: fmt.Sprintf("%s/oauth/token", g.URL), - RedirectURL: fmt.Sprintf("%s/authorize", httputil.GetURL(r)), + RedirectURL: fmt.Sprintf("%s/authorize", server.Config.Server.Host), //settings.Server.Scheme, settings.Server.Hostname), }, Transport: &http.Transport{ diff --git a/remote/gitlab3/gitlab.go b/remote/gitlab3/gitlab.go index 4d34cd31a..127525e8b 100644 --- a/remote/gitlab3/gitlab.go +++ b/remote/gitlab3/gitlab.go @@ -26,7 +26,7 @@ import ( "github.com/woodpecker-ci/woodpecker/model" "github.com/woodpecker-ci/woodpecker/remote" - "github.com/woodpecker-ci/woodpecker/shared/httputil" + "github.com/woodpecker-ci/woodpecker/server" "github.com/woodpecker-ci/woodpecker/shared/oauth2" "github.com/woodpecker-ci/woodpecker/remote/gitlab3/client" @@ -121,7 +121,7 @@ func (g *Gitlab) Login(res http.ResponseWriter, req *http.Request) (*model.User, Scope: DefaultScope, AuthURL: fmt.Sprintf("%s/oauth/authorize", g.URL), TokenURL: fmt.Sprintf("%s/oauth/token", g.URL), - RedirectURL: fmt.Sprintf("%s/authorize", httputil.GetURL(req)), + RedirectURL: fmt.Sprintf("%s/authorize", server.Config.Server.Host), } trans_ := &http.Transport{ @@ -631,7 +631,7 @@ func (g *Gitlab) Oauth2Transport(r *http.Request) *oauth2.Transport { Scope: DefaultScope, AuthURL: fmt.Sprintf("%s/oauth/authorize", g.URL), TokenURL: fmt.Sprintf("%s/oauth/token", g.URL), - RedirectURL: fmt.Sprintf("%s/authorize", httputil.GetURL(r)), + RedirectURL: fmt.Sprintf("%s/authorize", server.Config.Server.Host), //settings.Server.Scheme, settings.Server.Hostname), }, Transport: &http.Transport{ diff --git a/server/build.go b/server/build.go index db983467b..ab6a7b678 100644 --- a/server/build.go +++ b/server/build.go @@ -31,7 +31,6 @@ import ( "github.com/sirupsen/logrus" "github.com/woodpecker-ci/woodpecker/cncd/queue" "github.com/woodpecker-ci/woodpecker/remote" - "github.com/woodpecker-ci/woodpecker/shared/httputil" "github.com/woodpecker-ci/woodpecker/store" "github.com/woodpecker-ci/woodpecker/model" @@ -309,7 +308,7 @@ func PostApproval(c *gin.Context) { Netrc: netrc, Secs: secs, Regs: regs, - Link: httputil.GetURL(c.Request), + Link: Config.Server.Host, Yamls: yamls, Envs: envs, } @@ -518,7 +517,7 @@ func PostBuild(c *gin.Context) { Netrc: netrc, Secs: secs, Regs: regs, - Link: httputil.GetURL(c.Request), + Link: Config.Server.Host, Yamls: yamls, Envs: buildParams, } diff --git a/server/configFetcher.go b/server/configFetcher.go index a20eeabf5..5b79d1a53 100644 --- a/server/configFetcher.go +++ b/server/configFetcher.go @@ -15,6 +15,15 @@ type configFetcher struct { build *model.Build } +func NewConfigFetcher(remote remote.Remote, user *model.User, repo *model.Repo, build *model.Build) *configFetcher { + return &configFetcher{ + remote_: remote, + user: user, + repo: repo, + build: build, + } +} + func (cf *configFetcher) Fetch() ([]*remote.FileMeta, error) { for i := 0; i < 5; i++ { select { @@ -22,7 +31,7 @@ func (cf *configFetcher) Fetch() ([]*remote.FileMeta, error) { // either a file file, fileerr := cf.remote_.File(cf.user, cf.repo, cf.build, cf.repo.Config) if fileerr == nil { - return []*remote.FileMeta{&remote.FileMeta{ + return []*remote.FileMeta{{ Name: cf.repo.Config, Data: file, }}, nil @@ -43,7 +52,7 @@ func (cf *configFetcher) Fetch() ([]*remote.FileMeta, error) { return nil, fileerr } - return []*remote.FileMeta{&remote.FileMeta{ + return []*remote.FileMeta{{ Name: cf.repo.Config, Data: file, }}, nil diff --git a/server/configFetcher_test.go b/server/configFetcher_test.go index ac11ff880..cf1724834 100644 --- a/server/configFetcher_test.go +++ b/server/configFetcher_test.go @@ -1,10 +1,11 @@ -package server +package server_test import ( "testing" "github.com/woodpecker-ci/woodpecker/model" "github.com/woodpecker-ci/woodpecker/remote/github" + "github.com/woodpecker-ci/woodpecker/server" ) func TestFetchGithub(t *testing.T) { @@ -14,11 +15,11 @@ func TestFetchGithub(t *testing.T) { if err != nil { t.Fatal(err) } - configFetcher := &configFetcher{ - remote_: github, - user: &model.User{Token: "xxx"}, - repo: &model.Repo{Owner: "laszlocph", Name: "drone-multipipeline", Config: ".drone"}, - build: &model.Build{Commit: "89ab7b2d6bfb347144ac7c557e638ab402848fee"}, - } + configFetcher := server.NewConfigFetcher( + github, + &model.User{Token: "xxx"}, + &model.Repo{Owner: "laszlocph", Name: "drone-multipipeline", Config: ".drone"}, + &model.Build{Commit: "89ab7b2d6bfb347144ac7c557e638ab402848fee"}, + ) configFetcher.Fetch() } diff --git a/server/hook.go b/server/hook.go index b9abed9a4..4feb7a3b5 100644 --- a/server/hook.go +++ b/server/hook.go @@ -33,7 +33,6 @@ import ( "github.com/sirupsen/logrus" "github.com/woodpecker-ci/woodpecker/model" "github.com/woodpecker-ci/woodpecker/remote" - "github.com/woodpecker-ci/woodpecker/shared/httputil" "github.com/woodpecker-ci/woodpecker/shared/token" "github.com/woodpecker-ci/woodpecker/store" @@ -257,7 +256,7 @@ func PostHook(c *gin.Context) { Secs: secs, Regs: regs, Envs: envs, - Link: httputil.GetURL(c.Request), + Link: Config.Server.Host, Yamls: remoteYamlConfigs, } buildItems, err := b.Build() diff --git a/server/repo.go b/server/repo.go index 0288cb919..b313bc6d7 100644 --- a/server/repo.go +++ b/server/repo.go @@ -26,7 +26,6 @@ import ( "github.com/woodpecker-ci/woodpecker/model" "github.com/woodpecker-ci/woodpecker/remote" "github.com/woodpecker-ci/woodpecker/router/middleware/session" - "github.com/woodpecker-ci/woodpecker/shared/httputil" "github.com/woodpecker-ci/woodpecker/shared/token" "github.com/woodpecker-ci/woodpecker/store" ) @@ -75,7 +74,7 @@ func PostRepo(c *gin.Context) { link := fmt.Sprintf( "%s/hook?access_token=%s", - httputil.GetURL(c.Request), + Config.Server.Host, sig, ) @@ -203,7 +202,7 @@ func DeleteRepo(c *gin.Context) { } } - remote.Deactivate(user, repo, httputil.GetURL(c.Request)) + remote.Deactivate(user, repo, Config.Server.Host) c.JSON(200, repo) } @@ -221,7 +220,7 @@ func RepairRepo(c *gin.Context) { } // reconstruct the link - host := httputil.GetURL(c.Request) + host := Config.Server.Host link := fmt.Sprintf( "%s/hook?access_token=%s", host, @@ -307,7 +306,7 @@ func MoveRepo(c *gin.Context) { } // reconstruct the link - host := httputil.GetURL(c.Request) + host := Config.Server.Host link := fmt.Sprintf( "%s/hook?access_token=%s", host, diff --git a/shared/httputil/httputil.go b/shared/httputil/httputil.go index 011e23305..9aec3f9b3 100644 --- a/shared/httputil/httputil.go +++ b/shared/httputil/httputil.go @@ -38,65 +38,6 @@ func IsHttps(r *http.Request) bool { } } -// GetScheme is a helper function that evaluates the http.Request -// and returns the scheme, HTTP or HTTPS. It is able to detect, -// using the X-Forwarded-Proto, if the original request was HTTPS -// and routed through a reverse proxy with SSL termination. -func GetScheme(r *http.Request) string { - switch { - case r.URL.Scheme == "https": - return "https" - case r.TLS != nil: - return "https" - case strings.HasPrefix(r.Proto, "HTTPS"): - return "https" - case r.Header.Get("X-Forwarded-Proto") == "https": - return "https" - default: - return "http" - } -} - -// GetHost is a helper function that evaluates the http.Request -// and returns the hostname. It is able to detect, using the -// X-Forarded-For header, the original hostname when routed -// through a reverse proxy. -func GetHost(r *http.Request) string { - switch { - case len(r.Host) != 0: - return r.Host - case len(r.URL.Host) != 0: - return r.URL.Host - case len(r.Header.Get("X-Forwarded-For")) != 0: - return r.Header.Get("X-Forwarded-For") - case len(r.Header.Get("X-Host")) != 0: - return r.Header.Get("X-Host") - case len(r.Header.Get("XFF")) != 0: - return r.Header.Get("XFF") - case len(r.Header.Get("X-Real-IP")) != 0: - return r.Header.Get("X-Real-IP") - default: - return "localhost:8080" - } -} - -// GetURL is a helper function that evaluates the http.Request -// and returns the URL as a string. Only the scheme + hostname -// are included; the path is excluded. -func GetURL(r *http.Request) string { - return GetScheme(r) + "://" + GetHost(r) -} - -// GetCookie retrieves and verifies the cookie value. -func GetCookie(r *http.Request, name string) (value string) { - cookie, err := r.Cookie(name) - if err != nil { - return - } - value = cookie.Value - return -} - // SetCookie writes the cookie value. func SetCookie(w http.ResponseWriter, r *http.Request, name, value string) { cookie := http.Cookie{