Skip to content

Commit 9f6a4c3

Browse files
pudymodyfmartingr
pudymody
and
fmartingr
authored
feat: support proxy forward headers authentication (#1105)
* feat: Add SSO forward header * fix: Use domain layer * test: Some test * chore: Print new values when debugging * chore: Rename enabled envvar * fix: Wrongly parsing remote ip * fix: Always validate token. NPE on validateSession * fix: Dont overwrite token when sso * fix: Best effort to get ip. Parse as ip:port and then as ip * fix: Forgot to update handler version * fix: Forgot to commit changes * test: GetAccountByUsername * chore: Rename some variables * chore: return error from ssoAccount * refactor: Extract sso proxy auth to own middleware * fix: Dont panic if not sso account on legacy validate session * ci: gofmt --------- Co-authored-by: Felipe Martin <812088+fmartingr@users.noreply.github.com>
1 parent 24e06a5 commit 9f6a4c3

12 files changed

Lines changed: 378 additions & 37 deletions

File tree

‎docs/Configuration.md‎

Lines changed: 18 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -27,21 +27,24 @@ Most configuration can be set directly using environment variables or flags. The
2727

2828
### HTTP configuration variables
2929

30-
| Environment variable | Default | Required | Description |
31-
| ------------------------------------------ | ------- | -------- | ----------------------------------------------------- |
32-
| `SHIORI_HTTP_ENABLED` | True | No | Enable HTTP service |
33-
| `SHIORI_HTTP_PORT` | 8080 | No | Port number for the HTTP service |
34-
| `SHIORI_HTTP_ADDRESS` | : | No | Address for the HTTP service |
35-
| `SHIORI_HTTP_ROOT_PATH` | / | No | Root path for the HTTP service |
36-
| `SHIORI_HTTP_ACCESS_LOG` | True | No | Logging accessibility for HTTP requests |
37-
| `SHIORI_HTTP_SERVE_WEB_UI` | True | No | Serving Web UI via HTTP. Disable serves only the API. |
38-
| `SHIORI_HTTP_SECRET_KEY` | | **Yes** | Secret key for HTTP sessions. |
39-
| `SHIORI_HTTP_BODY_LIMIT` | 1024 | No | Limit for request body size |
40-
| `SHIORI_HTTP_READ_TIMEOUT` | 10s | No | Maximum duration for reading the entire request |
41-
| `SHIORI_HTTP_WRITE_TIMEOUT` | 10s | No | Maximum duration before timing out writes |
42-
| `SHIORI_HTTP_IDLE_TIMEOUT` | 10s | No | Maximum amount of time to wait for the next request |
43-
| `SHIORI_HTTP_DISABLE_KEEP_ALIVE` | true | No | Disable HTTP keep-alive connections |
44-
| `SHIORI_HTTP_DISABLE_PARSE_MULTIPART_FORM` | true | No | Disable pre-parsing of multipart form |
30+
| Environment variable | Default | Required | Description |
31+
| ------------------------------------------ | ------- | -------- | ----------------------------------------------------- |
32+
| `SHIORI_HTTP_ENABLED` | True | No | Enable HTTP service |
33+
| `SHIORI_HTTP_PORT` | 8080 | No | Port number for the HTTP service |
34+
| `SHIORI_HTTP_ADDRESS` | : | No | Address for the HTTP service |
35+
| `SHIORI_HTTP_ROOT_PATH` | / | No | Root path for the HTTP service |
36+
| `SHIORI_HTTP_ACCESS_LOG` | True | No | Logging accessibility for HTTP requests |
37+
| `SHIORI_HTTP_SERVE_WEB_UI` | True | No | Serving Web UI via HTTP. Disable serves only the API. |
38+
| `SHIORI_HTTP_SECRET_KEY` | | **Yes** | Secret key for HTTP sessions. |
39+
| `SHIORI_HTTP_BODY_LIMIT` | 1024 | No | Limit for request body size |
40+
| `SHIORI_HTTP_READ_TIMEOUT` | 10s | No | Maximum duration for reading the entire request |
41+
| `SHIORI_HTTP_WRITE_TIMEOUT` | 10s | No | Maximum duration before timing out writes |
42+
| `SHIORI_HTTP_IDLE_TIMEOUT` | 10s | No | Maximum amount of time to wait for the next request |
43+
| `SHIORI_HTTP_DISABLE_KEEP_ALIVE` | true | No | Disable HTTP keep-alive connections |
44+
| `SHIORI_HTTP_DISABLE_PARSE_MULTIPART_FORM` | true | No | Disable pre-parsing of multipart form |
45+
| `SHIORI_SSO_PROXY_AUTH_ENABLED` | false | No | Enable SSO Auth Proxy Header |
46+
| `SHIORI_SSO_PROXY_AUTH_HEADER_NAME` | Remote-User | No | List of CIDRs of trusted proxies |
47+
| `SHIORI_SSO_PROXY_AUTH_TRUSTED` | 10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16, fc00::/7 | No | List of CIDRs of trusted proxies |
4548

4649
### Storage Configuration
4750

‎internal/config/config.go‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,10 @@ type HttpConfig struct {
6565
IDLETimeout time.Duration `env:"HTTP_IDLE_TIMEOUT,default=10s"`
6666
DisableKeepAlive bool `env:"HTTP_DISABLE_KEEP_ALIVE,default=true"`
6767
DisablePreParseMultipartForm bool `env:"HTTP_DISABLE_PARSE_MULTIPART_FORM,default=true"`
68+
69+
SSOProxyAuth bool `env:"SSO_PROXY_AUTH_ENABLED,default=false"`
70+
SSOProxyAuthHeaderName string `env:"SSO_PROXY_AUTH_HEADER_NAME,default=Remote-User"`
71+
SSOProxyAuthTrusted []string `env:"SSO_PROXY_AUTH_TRUSTED,default=10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16, fc00::/7"`
6872
}
6973

7074
// SetDefaults sets the default values for the configuration
@@ -152,6 +156,9 @@ func (c *Config) DebugConfiguration(logger *logrus.Logger) {
152156
logger.Debugf(" SHIORI_HTTP_IDLE_TIMEOUT: %s", c.Http.IDLETimeout)
153157
logger.Debugf(" SHIORI_HTTP_DISABLE_KEEP_ALIVE: %t", c.Http.DisableKeepAlive)
154158
logger.Debugf(" SHIORI_HTTP_DISABLE_PARSE_MULTIPART_FORM: %t", c.Http.DisablePreParseMultipartForm)
159+
logger.Debugf(" SHIORI_SSO_PROXY_AUTH_ENABLED: %t", c.Http.SSOProxyAuth)
160+
logger.Debugf(" SHIORI_SSO_PROXY_AUTH_HEADER_NAME: %s", c.Http.SSOProxyAuthHeaderName)
161+
logger.Debugf(" SHIORI_SSO_PROXY_AUTH_TRUSTED: %v", c.Http.SSOProxyAuthTrusted)
155162
}
156163

157164
func (c *Config) IsValid() error {

‎internal/domains/accounts.go‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,24 @@ func (d *AccountsDomain) ListAccounts(ctx context.Context) ([]model.AccountDTO,
2929
return accountDTOs, nil
3030
}
3131

32+
func (d *AccountsDomain) GetAccountByUsername(ctx context.Context, username string) (*model.AccountDTO, error) {
33+
if username == "" {
34+
return nil, errors.New("empty username")
35+
}
36+
37+
accounts, err := d.deps.Database().ListAccounts(ctx, model.DBListAccountsOptions{
38+
Username: username,
39+
})
40+
if err != nil {
41+
return nil, fmt.Errorf("error getting accounts: %v", err)
42+
}
43+
if len(accounts) != 1 {
44+
return nil, fmt.Errorf("got none or more than one account by username: %s", username)
45+
}
46+
47+
return model.Ptr(accounts[0].ToDTO()), nil
48+
}
49+
3250
func (d *AccountsDomain) CreateAccount(ctx context.Context, account model.AccountDTO) (*model.AccountDTO, error) {
3351
if err := account.IsValidCreate(); err != nil {
3452
return nil, err

‎internal/domains/accounts_test.go‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,30 @@ func TestAccountDomainsListAccounts(t *testing.T) {
3838
})
3939
}
4040

41+
func TestAccountDomainsGetAccountByUsername(t *testing.T) {
42+
logger := logrus.New()
43+
_, deps := testutil.GetTestConfigurationAndDependencies(t, context.TODO(), logger)
44+
45+
t.Run("empty", func(t *testing.T) {
46+
account, err := deps.Domains().Accounts().GetAccountByUsername(context.Background(), "")
47+
require.Error(t, err)
48+
require.Nil(t, account)
49+
})
50+
51+
t.Run("account found", func(t *testing.T) {
52+
_, err := deps.Domains().Accounts().CreateAccount(context.TODO(), model.AccountDTO{
53+
Username: "user1",
54+
Password: "password1",
55+
})
56+
require.NoError(t, err)
57+
58+
account, err := deps.Domains().Accounts().GetAccountByUsername(context.Background(), "user1")
59+
require.NoError(t, err)
60+
require.NotNil(t, account)
61+
require.Equal(t, "user1", account.Username)
62+
})
63+
}
64+
4165
func TestAccountDomainCreateAccount(t *testing.T) {
4266
logger := logrus.New()
4367
_, deps := testutil.GetTestConfigurationAndDependencies(t, context.TODO(), logger)

‎internal/http/middleware/auth.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,10 @@ func NewAuthMiddleware(deps model.Dependencies) *AuthMiddleware {
2121
}
2222

2323
func (m *AuthMiddleware) OnRequest(deps model.Dependencies, c model.WebContext) error {
24+
if c.UserIsLogged() {
25+
return nil
26+
}
27+
2428
token := getTokenFromHeader(c.Request())
2529
if token == "" {
2630
token = getTokenFromCookie(c.Request())

‎internal/http/middleware/auth_sso_proxy.go‎

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
package middleware
2+
3+
import (
4+
"errors"
5+
"net"
6+
7+
"github.com/go-shiori/shiori/internal/model"
8+
)
9+
10+
// AuthMiddleware handles authentication for incoming request by checking the token
11+
// from the Authorization header or the token cookie and setting the account in the
12+
// request context.
13+
type AuthSSOProxyMiddleware struct {
14+
deps model.Dependencies
15+
16+
trustedIPs []*net.IPNet
17+
}
18+
19+
func NewAuthSSOProxyMiddleware(deps model.Dependencies) *AuthSSOProxyMiddleware {
20+
plainIPs := deps.Config().Http.SSOProxyAuthTrusted
21+
trustedIPs := make([]*net.IPNet, len(plainIPs))
22+
for i, ip := range plainIPs {
23+
_, ipNet, err := net.ParseCIDR(ip)
24+
if err != nil {
25+
deps.Logger().WithError(err).WithField("ip", ip).Error("Failed to parse trusted ip cidr")
26+
continue
27+
}
28+
29+
trustedIPs[i] = ipNet
30+
}
31+
32+
return &AuthSSOProxyMiddleware{
33+
deps: deps,
34+
trustedIPs: trustedIPs,
35+
}
36+
}
37+
38+
func (m *AuthSSOProxyMiddleware) OnRequest(deps model.Dependencies, c model.WebContext) error {
39+
if c.UserIsLogged() {
40+
return nil
41+
}
42+
43+
account, err := m.ssoAccount(deps, c)
44+
if err != nil {
45+
deps.Logger().
46+
WithError(err).
47+
WithField("remote_addr", c.Request().RemoteAddr).
48+
WithField("request_id", c.GetRequestID()).
49+
Error("getting sso account")
50+
return nil
51+
}
52+
if account != nil {
53+
c.SetAccount(account)
54+
return nil
55+
}
56+
57+
return nil
58+
}
59+
60+
func (m *AuthSSOProxyMiddleware) ssoAccount(deps model.Dependencies, c model.WebContext) (*model.AccountDTO, error) {
61+
if !deps.Config().Http.SSOProxyAuth {
62+
return nil, nil
63+
}
64+
65+
remoteAddr := c.Request().RemoteAddr
66+
ip, _, err := net.SplitHostPort(remoteAddr)
67+
if err != nil {
68+
var addrErr *net.AddrError
69+
if errors.As(err, &addrErr) && addrErr.Err == "missing port in address" {
70+
ip = remoteAddr
71+
} else {
72+
return nil, err
73+
}
74+
}
75+
requestIP := net.ParseIP(ip)
76+
if !m.isTrustedIP(requestIP) {
77+
return nil, errors.New("remoteAddr is not a trusted ip")
78+
}
79+
80+
headerName := deps.Config().Http.SSOProxyAuthHeaderName
81+
userName := c.Request().Header.Get(headerName)
82+
if userName == "" {
83+
return nil, nil
84+
}
85+
86+
account, err := deps.Domains().Accounts().GetAccountByUsername(c.Request().Context(), userName)
87+
if err != nil {
88+
return nil, err
89+
}
90+
91+
return account, nil
92+
}
93+
func (m *AuthSSOProxyMiddleware) isTrustedIP(ip net.IP) bool {
94+
for _, net := range m.trustedIPs {
95+
if ok := net.Contains(ip); ok {
96+
return true
97+
}
98+
}
99+
return false
100+
}
101+
102+
func (m *AuthSSOProxyMiddleware) OnResponse(deps model.Dependencies, c model.WebContext) error {
103+
return nil
104+
}

‎internal/http/middleware/auth_sso_proxy_test.go‎

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
package middleware
2+
3+
import (
4+
"context"
5+
"net/http"
6+
"net/http/httptest"
7+
"testing"
8+
9+
"github.com/go-shiori/shiori/internal/http/webcontext"
10+
"github.com/go-shiori/shiori/internal/model"
11+
"github.com/go-shiori/shiori/internal/testutil"
12+
"github.com/sirupsen/logrus"
13+
"github.com/stretchr/testify/require"
14+
)
15+
16+
func TestAuthMiddlewareWithSSO(t *testing.T) {
17+
logger := logrus.New()
18+
_, deps := testutil.GetTestConfigurationAndDependencies(t, context.TODO(), logger)
19+
deps.Config().Http.SSOProxyAuth = true
20+
21+
account, err := deps.Domains().Accounts().CreateAccount(context.TODO(), model.AccountDTO{
22+
ID: model.DBID(98),
23+
Username: "test_username",
24+
Password: "super_secure_password",
25+
})
26+
require.NoError(t, err)
27+
28+
t.Run("test no authorization method", func(t *testing.T) {
29+
w := httptest.NewRecorder()
30+
r := httptest.NewRequest(http.MethodGet, "/", nil)
31+
c := webcontext.NewWebContext(w, r)
32+
33+
middleware := NewAuthSSOProxyMiddleware(deps)
34+
err := middleware.OnRequest(deps, c)
35+
require.NoError(t, err)
36+
require.Nil(t, c.GetAccount())
37+
})
38+
39+
t.Run("test untrusted ip", func(t *testing.T) {
40+
w := httptest.NewRecorder()
41+
r := httptest.NewRequest(http.MethodGet, "/", nil)
42+
r.RemoteAddr = "invalid-ip"
43+
c := webcontext.NewWebContext(w, r)
44+
45+
middleware := NewAuthSSOProxyMiddleware(deps)
46+
err := middleware.OnRequest(deps, c)
47+
require.NoError(t, err)
48+
require.Nil(t, c.GetAccount())
49+
})
50+
51+
t.Run("test empty header", func(t *testing.T) {
52+
w := httptest.NewRecorder()
53+
r := httptest.NewRequest(http.MethodGet, "/", nil)
54+
r.RemoteAddr = "10.0.0.3"
55+
c := webcontext.NewWebContext(w, r)
56+
57+
middleware := NewAuthSSOProxyMiddleware(deps)
58+
err := middleware.OnRequest(deps, c)
59+
require.NoError(t, err)
60+
require.Nil(t, c.GetAccount())
61+
})
62+
63+
t.Run("test invalid sso username", func(t *testing.T) {
64+
w := httptest.NewRecorder()
65+
r := httptest.NewRequest(http.MethodGet, "/", nil)
66+
r.RemoteAddr = "10.0.0.3"
67+
r.Header.Add("Remote-User", "username")
68+
c := webcontext.NewWebContext(w, r)
69+
70+
middleware := NewAuthSSOProxyMiddleware(deps)
71+
err := middleware.OnRequest(deps, c)
72+
require.NoError(t, err)
73+
require.Nil(t, c.GetAccount())
74+
})
75+
76+
t.Run("test sso login", func(t *testing.T) {
77+
w := httptest.NewRecorder()
78+
r := httptest.NewRequest(http.MethodGet, "/", nil)
79+
r.RemoteAddr = "10.0.0.3"
80+
r.Header.Add("Remote-User", account.Username)
81+
c := webcontext.NewWebContext(w, r)
82+
83+
middleware := NewAuthSSOProxyMiddleware(deps)
84+
err := middleware.OnRequest(deps, c)
85+
require.NoError(t, err)
86+
require.NotNil(t, c.GetAccount())
87+
})
88+
89+
t.Run("test sso login ip:port", func(t *testing.T) {
90+
w := httptest.NewRecorder()
91+
r := httptest.NewRequest(http.MethodGet, "/", nil)
92+
r.RemoteAddr = "10.0.0.3:65342"
93+
r.Header.Add("Remote-User", account.Username)
94+
c := webcontext.NewWebContext(w, r)
95+
96+
middleware := NewAuthSSOProxyMiddleware(deps)
97+
err := middleware.OnRequest(deps, c)
98+
require.NoError(t, err)
99+
require.NotNil(t, c.GetAccount())
100+
})
101+
}

‎internal/http/server.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,10 @@ func (s *HttpServer) Setup(cfg *config.Config, deps *dependencies.Dependencies)
3333

3434
globalMiddleware := []model.HttpMiddleware{}
3535

36+
if cfg.Http.SSOProxyAuth {
37+
globalMiddleware = append(globalMiddleware, middleware.NewAuthSSOProxyMiddleware(deps))
38+
}
39+
3640
// Add message response middleware if legacy message response is enabled
3741
globalMiddleware = append(globalMiddleware, []model.HttpMiddleware{
3842
middleware.NewMessageResponseMiddleware(deps),

‎internal/model/domains.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ type AuthDomain interface {
3131

3232
type AccountsDomain interface {
3333
ListAccounts(ctx context.Context) ([]AccountDTO, error)
34+
GetAccountByUsername(ctx context.Context, username string) (*AccountDTO, error)
3435
CreateAccount(ctx context.Context, account AccountDTO) (*AccountDTO, error)
3536
UpdateAccount(ctx context.Context, account AccountDTO) (*AccountDTO, error)
3637
DeleteAccount(ctx context.Context, id int) error

‎internal/view/index.html‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -160,19 +160,15 @@
160160
},
161161

162162
onLoginSuccess() {
163-
this.loadSetting();
164163
this.loadAccount();
164+
this.loadSetting();
165165
this.isLoggedIn = true;
166166
},
167167

168168
async validateSession() {
169169
const token = localStorage.getItem("shiori-token");
170170
const account = localStorage.getItem("shiori-account");
171171

172-
if (!(token && account)) {
173-
return false;
174-
}
175-
176172
try {
177173
const response = await fetch(new URL("api/v1/auth/me", document.baseURI), {
178174
headers: {
@@ -184,6 +180,11 @@
184180
throw new Error('Invalid session');
185181
}
186182

183+
const responseJSON = await response.json();
184+
localStorage.setItem(
185+
"shiori-account",
186+
JSON.stringify(responseJSON.message),
187+
);
187188
return true;
188189
} catch (err) {
189190
// Clear invalid session data

0 commit comments

Comments
 (0)