Skip to content

Commit

Permalink
Merge remote-tracking branch 'grafana/master'
Browse files Browse the repository at this point in the history
* grafana/master:
  changelog: update
  changelog: update
  changelog: add notes about closing grafana#12438
  alerting: only log when screenshot been uploaded
  fixes typos
  changelog: add notes about closing grafana#12444
  Revert "auth proxy: use real ip when validating white listed ip's"
  changelog: adds note for grafana#11892
  changelog: add notes about closing grafana#12430
  fix footer css issue
  Karma to Jest: 3 test files (grafana#12414)
  fix: log close/flush was done too early, before server shutdown log message was called, fixes grafana#12438
  Karma to Jest: value_select_dropdown (grafana#12435)
  support passing api token in Basic auth password (grafana#12416)
  Add disabled styles for checked checkbox (grafana#12422)
  Make pre/postfix coloring checkboxes inactive when gauge is active
  Add options to colorize prefix and postfix in singlestat
  • Loading branch information
ryantxu committed Jun 29, 2018
2 parents b3b1e70 + cb706bd commit b9c716d
Show file tree
Hide file tree
Showing 23 changed files with 521 additions and 547 deletions.
11 changes: 10 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,15 @@

* **Api**: Delete nonexistent datasource should return 404 [#12313](https://github.com/grafana/grafana/issues/12313), thx [@AustinWinstanley](https://github.com/AustinWinstanley)
* **Dashboard**: Fix selecting current dashboard from search should not reload dashboard [#12248](https://github.com/grafana/grafana/issues/12248)
* **Singlestat**: Make colorization of prefix and postfix optional in singlestat [#11892](https://github.com/grafana/grafana/pull/11892), thx [@ApsOps](https://github.com/ApsOps)

# 5.2.1 (2018-06-29)

### Minor

* **Auth Proxy**: Important security fix for whitelist of IP address feature [#12444](https://github.com/grafana/grafana/pull/12444)
* **UI**: Fix - Grafana footer overlapping page [#12430](https://github.com/grafana/grafana/issues/12430)
* **Logging**: Errors should be reported before crashing [#12438](https://github.com/grafana/grafana/issues/12438)

# 5.2.0-stable (2018-06-27)

Expand Down Expand Up @@ -1328,7 +1337,7 @@ Grafana 2.x is fundamentally different from 1.x; it now ships with an integrated
**New features**
- [Issue #1623](https://github.com/grafana/grafana/issues/1623). Share Dashboard: Dashboard snapshot sharing (dash and data snapshot), save to local or save to public snapshot dashboard snapshots.raintank.io site
- [Issue #1622](https://github.com/grafana/grafana/issues/1622). Share Panel: The share modal now has an embed option, gives you an iframe that you can use to embedd a single graph on another web site
- [Issue #718](https://github.com/grafana/grafana/issues/718). Dashboard: When saving a dashboard and another user has made changes in between the user is promted with a warning if he really wants to overwrite the other's changes
- [Issue #718](https://github.com/grafana/grafana/issues/718). Dashboard: When saving a dashboard and another user has made changes in between the user is prompted with a warning if he really wants to overwrite the other's changes
- [Issue #1331](https://github.com/grafana/grafana/issues/1331). Graph & Singlestat: New axis/unit format selector and more units (kbytes, Joule, Watt, eV), and new design for graph axis & grid tab and single stat options tab views
- [Issue #1241](https://github.com/grafana/grafana/issues/1242). Timepicker: New option in timepicker (under dashboard settings), to change ``now`` to be for example ``now-1m``, useful when you want to ignore last minute because it contains incomplete data
- [Issue #171](https://github.com/grafana/grafana/issues/171). Panel: Different time periods, panels can override dashboard relative time and/or add a time shift
Expand Down
8 changes: 8 additions & 0 deletions docs/sources/http_api/auth.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,14 @@ Authorization: Bearer eyJrIjoiT0tTcG1pUlY2RnVKZTFVaDFsNFZXdE9ZWmNrMkZYbk

The `Authorization` header value should be `Bearer <your api key>`.

The API Token can also be passed as a Basic authorization password with the special username `api_key`:

curl example:
```bash
?curl http://api_key:eyJrIjoiT0tTcG1pUlY2RnVKZTFVaDFsNFZXdE9ZWmNrMkZYbk@localhost:3000/api/org
{"id":1,"name":"Main Org."}
```

# Auth HTTP resources / actions

## Api Keys
Expand Down
4 changes: 0 additions & 4 deletions pkg/cmd/grafana-server/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import (
"net/http"
_ "net/http/pprof"

"github.com/grafana/grafana/pkg/log"
"github.com/grafana/grafana/pkg/metrics"
"github.com/grafana/grafana/pkg/setting"

Expand Down Expand Up @@ -88,9 +87,6 @@ func main() {

err := server.Run()

trace.Stop()
log.Close()

server.Exit(err)
}

Expand Down
2 changes: 2 additions & 0 deletions pkg/cmd/grafana-server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,8 @@ func (g *GrafanaServerImpl) Exit(reason error) {
}

g.log.Error("Server shutdown", "reason", reason)

log.Close()
os.Exit(code)
}

Expand Down
6 changes: 6 additions & 0 deletions pkg/middleware/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
m "github.com/grafana/grafana/pkg/models"
"github.com/grafana/grafana/pkg/services/session"
"github.com/grafana/grafana/pkg/setting"
"github.com/grafana/grafana/pkg/util"
)

type AuthOptions struct {
Expand All @@ -34,6 +35,11 @@ func getApiKey(c *m.ReqContext) string {
return key
}

username, password, err := util.DecodeBasicAuthHeader(header)
if err == nil && username == "api_key" {
return password
}

return ""
}

Expand Down
20 changes: 8 additions & 12 deletions pkg/middleware/auth_proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package middleware

import (
"fmt"
"net"
"net/mail"
"reflect"
"strings"
Expand All @@ -28,7 +29,7 @@ func initContextWithAuthProxy(ctx *m.ReqContext, orgID int64) bool {
}

// if auth proxy ip(s) defined, check if request comes from one of those
if err := checkAuthenticationProxy(ctx.RemoteAddr(), proxyHeaderValue); err != nil {
if err := checkAuthenticationProxy(ctx.Req.RemoteAddr, proxyHeaderValue); err != nil {
ctx.Handle(407, "Proxy authentication required", err)
return true
}
Expand Down Expand Up @@ -196,23 +197,18 @@ func checkAuthenticationProxy(remoteAddr string, proxyHeaderValue string) error
return nil
}

// Multiple ip addresses? Right-most IP address is the IP address of the most recent proxy
if strings.Contains(remoteAddr, ",") {
sourceIPs := strings.Split(remoteAddr, ",")
remoteAddr = strings.TrimSpace(sourceIPs[len(sourceIPs)-1])
}

remoteAddr = strings.TrimPrefix(remoteAddr, "[")
remoteAddr = strings.TrimSuffix(remoteAddr, "]")

proxies := strings.Split(setting.AuthProxyWhitelist, ",")
sourceIP, _, err := net.SplitHostPort(remoteAddr)
if err != nil {
return err
}

// Compare allowed IP addresses to actual address
for _, proxyIP := range proxies {
if remoteAddr == strings.TrimSpace(proxyIP) {
if sourceIP == strings.TrimSpace(proxyIP) {
return nil
}
}

return fmt.Errorf("Request for user (%s) from %s is not from the authentication proxy", proxyHeaderValue, remoteAddr)
return fmt.Errorf("Request for user (%s) from %s is not from the authentication proxy", proxyHeaderValue, sourceIP)
}
81 changes: 24 additions & 57 deletions pkg/middleware/middleware_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ func TestMiddlewareContext(t *testing.T) {

setting.BasicAuthEnabled = true
authHeader := util.GetBasicAuthHeader("myUser", "myPass")
sc.fakeReq("GET", "/").withAuthoriziationHeader(authHeader).exec()
sc.fakeReq("GET", "/").withAuthorizationHeader(authHeader).exec()

Convey("Should init middleware context with user", func() {
So(sc.context.IsSignedIn, ShouldEqual, true)
Expand Down Expand Up @@ -128,6 +128,28 @@ func TestMiddlewareContext(t *testing.T) {
})
})

middlewareScenario("Valid api key via Basic auth", func(sc *scenarioContext) {
keyhash := util.EncodePassword("v5nAwpMafFP6znaS4urhdWDLS5511M42", "asd")

bus.AddHandler("test", func(query *m.GetApiKeyByNameQuery) error {
query.Result = &m.ApiKey{OrgId: 12, Role: m.ROLE_EDITOR, Key: keyhash}
return nil
})

authHeader := util.GetBasicAuthHeader("api_key", "eyJrIjoidjVuQXdwTWFmRlA2em5hUzR1cmhkV0RMUzU1MTFNNDIiLCJuIjoiYXNkIiwiaWQiOjF9")
sc.fakeReq("GET", "/").withAuthorizationHeader(authHeader).exec()

Convey("Should return 200", func() {
So(sc.resp.Code, ShouldEqual, 200)
})

Convey("Should init middleware context", func() {
So(sc.context.IsSignedIn, ShouldEqual, true)
So(sc.context.OrgId, ShouldEqual, 12)
So(sc.context.OrgRole, ShouldEqual, m.ROLE_EDITOR)
})
})

middlewareScenario("UserId in session", func(sc *scenarioContext) {

sc.fakeReq("GET", "/").handler(func(c *m.ReqContext) {
Expand Down Expand Up @@ -293,61 +315,6 @@ func TestMiddlewareContext(t *testing.T) {
})
})

middlewareScenario("When auth_proxy is enabled and request has X-Forwarded-For that is not trusted", func(sc *scenarioContext) {
setting.AuthProxyEnabled = true
setting.AuthProxyHeaderName = "X-WEBAUTH-USER"
setting.AuthProxyHeaderProperty = "username"
setting.AuthProxyWhitelist = "192.168.1.1, 2001::23"

bus.AddHandler("test", func(query *m.GetSignedInUserQuery) error {
query.Result = &m.SignedInUser{OrgId: 4, UserId: 33}
return nil
})

bus.AddHandler("test", func(cmd *m.UpsertUserCommand) error {
cmd.Result = &m.User{Id: 33}
return nil
})

sc.fakeReq("GET", "/")
sc.req.Header.Add("X-WEBAUTH-USER", "torkelo")
sc.req.Header.Add("X-Forwarded-For", "client-ip, 192.168.1.1, 192.168.1.2")
sc.exec()

Convey("should return 407 status code", func() {
So(sc.resp.Code, ShouldEqual, 407)
So(sc.resp.Body.String(), ShouldContainSubstring, "Request for user (torkelo) from 192.168.1.2 is not from the authentication proxy")
})
})

middlewareScenario("When auth_proxy is enabled and request has X-Forwarded-For that is trusted", func(sc *scenarioContext) {
setting.AuthProxyEnabled = true
setting.AuthProxyHeaderName = "X-WEBAUTH-USER"
setting.AuthProxyHeaderProperty = "username"
setting.AuthProxyWhitelist = "192.168.1.1, 2001::23"

bus.AddHandler("test", func(query *m.GetSignedInUserQuery) error {
query.Result = &m.SignedInUser{OrgId: 4, UserId: 33}
return nil
})

bus.AddHandler("test", func(cmd *m.UpsertUserCommand) error {
cmd.Result = &m.User{Id: 33}
return nil
})

sc.fakeReq("GET", "/")
sc.req.Header.Add("X-WEBAUTH-USER", "torkelo")
sc.req.Header.Add("X-Forwarded-For", "client-ip, 192.168.1.2, 192.168.1.1")
sc.exec()

Convey("Should init context with user info", func() {
So(sc.context.IsSignedIn, ShouldBeTrue)
So(sc.context.UserId, ShouldEqual, 33)
So(sc.context.OrgId, ShouldEqual, 4)
})
})

middlewareScenario("When session exists for previous user, create a new session", func(sc *scenarioContext) {
setting.AuthProxyEnabled = true
setting.AuthProxyHeaderName = "X-WEBAUTH-USER"
Expand Down Expand Up @@ -473,7 +440,7 @@ func (sc *scenarioContext) withInvalidApiKey() *scenarioContext {
return sc
}

func (sc *scenarioContext) withAuthoriziationHeader(authHeader string) *scenarioContext {
func (sc *scenarioContext) withAuthorizationHeader(authHeader string) *scenarioContext {
sc.authHeader = authHeader
return sc
}
Expand Down
5 changes: 4 additions & 1 deletion pkg/services/alerting/notifier.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,10 @@ func (n *notificationService) uploadImage(context *EvalContext) (err error) {
return err
}

n.log.Info("uploaded", "url", context.ImagePublicUrl)
if context.ImagePublicUrl != "" {
n.log.Info("uploaded screenshot of alert to external image store", "url", context.ImagePublicUrl)
}

return nil
}

Expand Down
4 changes: 2 additions & 2 deletions public/app/core/directives/value_select_dropdown.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ export class ValueSelectDropdownCtrl {
tagValuesPromise = this.$q.when(tag.values);
}

tagValuesPromise.then(values => {
return tagValuesPromise.then(values => {
tag.values = values;
tag.valuesText = values.join(' + ');
_.each(this.options, option => {
Expand Down Expand Up @@ -132,7 +132,7 @@ export class ValueSelectDropdownCtrl {
this.highlightIndex = (this.highlightIndex + direction) % this.search.options.length;
}

selectValue(option, event, commitChange, excludeOthers) {
selectValue(option, event, commitChange?, excludeOthers?) {
if (!option) {
return;
}
Expand Down
Loading

0 comments on commit b9c716d

Please sign in to comment.