fix: Require guests to login (#678)

This commit is contained in:
James Read 2025-10-30 13:22:39 +00:00 committed by GitHub
commit e0167c9e42
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
14 changed files with 112 additions and 94 deletions

1
.gitignore vendored
View File

@ -16,3 +16,4 @@ integration-tests/screenshots/
webui/
server.log
OliveTin
integration-tests/configs/authRequireGuestsToLogin/sessions.yaml

View File

@ -26,7 +26,7 @@ If you are looking for OliveTin's AI policy, you can find it in `AI.md`.
### Test Notes and Gotchas
- The top-level Makefile does not expose `unittests`; use `cd service && make unittests`.
- Connect RPC API must be mounted correctly; in tests, create the handler via `GetNewHandler(ex)` and serve under `/api/`.
- Frontend “ready” state: the app sets `document.body` attribute `initial-marshal-complete="true"` when loaded. Integration helpers wait for this before selecting elements.
- Frontend “ready” state: the app sets `document.body` attribute `loaded-dashboard="<name>"` when loading a dashboard. Integration helpers that test dashboard functionality wait for this before selecting elements. Certain conditions enforcing login will mean that this attribute is not set until a user is logged in.
- Modern UI uses Vue components:
- Action buttons are rendered as `.action-button button`.
- Logs and Diagnostics are Vue router links available via `/logs` and `/diagnostics`.
@ -64,6 +64,6 @@ If you are looking for OliveTin's AI policy, you can find it in `AI.md`.
### Troubleshooting
- API tests failing with content-type errors: ensure Connect handler is served under `/api/` and the client targets that base URL.
- Executor panics: check for nil `Binding/Action` and add guards in step functions.
- Integration timeouts: wait for `initial-marshal-complete` and use selectors matching the Vue UI.
- Integration timeouts: wait for `loaded-dashboard` and use selectors matching the Vue UI.

View File

@ -1378,6 +1378,11 @@ export declare type InitResponse = Message<"olivetin.api.v1.InitResponse"> & {
* @generated from field: bool show_log_list = 22;
*/
showLogList: boolean;
/**
* @generated from field: bool login_required = 23;
*/
loginRequired: boolean;
};
/**

File diff suppressed because one or more lines are too long

View File

@ -59,6 +59,7 @@
<script setup>
import { ref, onMounted } from 'vue';
import { useRouter } from 'vue-router';
import Sidebar from 'picocrank/vue/components/Sidebar.vue';
import Header from 'picocrank/vue/components/Header.vue';
import { HugeiconsIcon } from '@hugeicons/vue'
@ -67,6 +68,8 @@ import { UserCircle02Icon } from '@hugeicons/core-free-icons'
import { DashboardSquare01Icon } from '@hugeicons/core-free-icons'
import logoUrl from '../../OliveTinLogo.png';
const router = useRouter();
const sidebar = ref(null);
const username = ref('guest');
const isLoggedIn = ref(false);
@ -107,7 +110,14 @@ async function requestInit() {
try {
const initResponse = await window.client.init({})
// Store init response first so the login view can read options (e.g., authLocalLogin)
window.initResponse = initResponse
// Check if login is required and redirect if so (after storing initResponse)
if (initResponse.loginRequired) {
router.push('/login')
return
}
window.initError = false
window.initErrorMessage = ''
window.initCompleted = true

View File

@ -13,6 +13,10 @@
<dd v-if="userProvider !== 'system'">{{ userProvider }}</dd>
<dt v-if="usergroup">Group</dt>
<dd v-if="usergroup">{{ usergroup }}</dd>
<dt v-if="acls && acls.length > 0">Matched ACLs</dt>
<dd v-if="acls && acls.length > 0">
<span class="acl-tag" v-for="(acl, idx) in acls" :key="`acl-${idx}`">{{ acl }}</span>
</dd>
</dl>
<div class="user-actions">
@ -38,6 +42,7 @@ const username = ref('guest')
const userProvider = ref('system')
const usergroup = ref('')
const loggingOut = ref(false)
const acls = ref([])
function updateUserInfo() {
if (window.initResponse) {
@ -48,6 +53,20 @@ function updateUserInfo() {
}
}
async function fetchWhoAmI() {
try {
const res = await window.client.whoAmI({})
acls.value = res.acls || []
// Update usergroup from authoritative WhoAmI response
if (res.usergroup) {
usergroup.value = res.usergroup
}
} catch (e) {
console.warn('Failed to fetch WhoAmI for ACLs', e)
acls.value = []
}
}
async function handleLogout() {
loggingOut.value = true
@ -70,8 +89,12 @@ async function handleLogout() {
console.error('Failed to reinitialize after logout:', initErr)
}
// Redirect to home page
// Redirect based on init response: if login is required, go to login page
if (window.initResponse && window.initResponse.loginRequired) {
router.push('/login')
} else {
router.push('/')
}
} catch (err) {
console.error('Logout error:', err)
} finally {
@ -83,13 +106,8 @@ let watchInterval = null
onMounted(() => {
updateUserInfo()
fetchWhoAmI()
// Watch for changes to init response
watchInterval = setInterval(() => {
if (window.initResponse) {
updateUserInfo()
}
}, 1000)
})
onUnmounted(() => {
@ -124,6 +142,16 @@ section {
gap: 1rem;
}
.acl-tag {
display: inline-block;
background: var(--section-background);
border: 1px solid var(--border-color);
border-radius: 0.25rem;
padding: 0.1rem 0.4rem;
margin: 0 0.25rem 0.25rem 0;
font-size: 0.85rem;
}
.button {
padding: 0.75rem 1.5rem;
border-radius: 4px;

View File

@ -14,9 +14,32 @@ authRequireGuestsToLogin: true
authLocalUsers:
enabled: true
users:
- username: "testuser"
usergroup: "admin"
password: "testpass123"
- username: "alice"
usergroup: "admins"
password: "$argon2id$v=19$m=65536,t=4,p=6$ORxyZZGW6E3FWZnbQmHJ9Q$BzIOWeXry/BZ6+JV1T4UASBnebVLB9QJ4f5TmUPXsg4" # notsecret: password
- username: "bob"
usergroup: "users"
password: "$argon2id$v=19$m=65536,t=4,p=6$ORxyZZGW6E3FWZnbQmHJ9Q$BzIOWeXry/BZ6+JV1T4UASBnebVLB9QJ4f5TmUPXsg4" # notsecret: password
accessControlLists:
- name: "admin"
matchUsergroups: ["admins"]
addToEveryAction: true
permissions:
view: true
exec: true
logs: true
kill: true
- name: "users"
matchUsergroups: ["users"]
addToEveryAction: true
permissions:
view: true
exec: false
logs: false
kill: false
# Simple actions for testing
actions:

View File

@ -21,39 +21,19 @@ describe('config: authRequireGuestsToLogin', function () {
takeScreenshotOnFailure(this.currentTest, webdriver);
});
it('Server starts successfully with authRequireGuestsToLogin enabled', async function () {
await webdriver.get(runner.baseUrl())
await webdriver.wait(until.titleContains('OliveTin'), 10000)
const title = await webdriver.getTitle()
expect(title).to.contain('OliveTin')
console.log('✓ Server started successfully with authRequireGuestsToLogin enabled')
})
it('Guest is redirected to login', async function () {
// Don't use getRootAndWait here because we want to test the redirect, and getRootAndWait waits for the dashboard to load
it('Guest user is blocked from accessing the web UI', async function () {
await webdriver.get(runner.baseUrl())
// Wait for the page to finish loading
await webdriver.wait(until.elementLocated(By.css('body')), 10000)
await new Promise(resolve => setTimeout(resolve, 3000))
await webdriver.wait(until.urlContains('/login'), 10000)
// The page should redirect or show an error because guest is not allowed
// We can't directly test the API from Selenium, but we can verify the page behavior
const currentUrl = await webdriver.getCurrentUrl()
console.log('Current URL:', currentUrl)
// Verify login UI elements are present
const loginElements = await webdriver.findElements(By.css('form.local-login-form, .login-oauth2, .login-disabled'))
expect(loginElements.length).to.be.greaterThan(0)
// At minimum, we verify the server responds
const pageText = await webdriver.findElement(By.tagName('body')).getText()
console.log('✓ Page loaded, guest behavior verified')
})
console.log('✓ Login page loaded correctly')
it('Authenticated user can login and access the dashboard', async function () {
await webdriver.get(runner.baseUrl())
// Check if there's a login link or login page
// This is a simplified test since we can't easily test the full auth flow from Selenium
const bodyText = await webdriver.findElement(By.tagName('body')).getText()
console.log('Page content preview:', bodyText.substring(0, 200))
console.log('✓ Authenticated user flow verified')
})
})

View File

@ -319,6 +319,7 @@ message InitResponse {
string banner_css = 20;
bool show_diagnostics = 21;
bool show_log_list = 22;
bool login_required = 23;
}
message AdditionalLink {

View File

@ -3139,6 +3139,7 @@ type InitResponse struct {
BannerCss string `protobuf:"bytes,20,opt,name=banner_css,json=bannerCss,proto3" json:"banner_css,omitempty"`
ShowDiagnostics bool `protobuf:"varint,21,opt,name=show_diagnostics,json=showDiagnostics,proto3" json:"show_diagnostics,omitempty"`
ShowLogList bool `protobuf:"varint,22,opt,name=show_log_list,json=showLogList,proto3" json:"show_log_list,omitempty"`
LoginRequired bool `protobuf:"varint,23,opt,name=login_required,json=loginRequired,proto3" json:"login_required,omitempty"`
unknownFields protoimpl.UnknownFields
sizeCache protoimpl.SizeCache
}
@ -3327,6 +3328,13 @@ func (x *InitResponse) GetShowLogList() bool {
return false
}
func (x *InitResponse) GetLoginRequired() bool {
if x != nil {
return x.LoginRequired
}
return false
}
type AdditionalLink struct {
state protoimpl.MessageState `protogen:"open.v1"`
Title string `protobuf:"bytes,1,opt,name=title,proto3" json:"title,omitempty"`
@ -3962,7 +3970,7 @@ const file_olivetin_api_v1_olivetin_proto_rawDesc = "" +
"\x16GetDiagnosticsResponse\x12 \n" +
"\vSshFoundKey\x18\x01 \x01(\tR\vSshFoundKey\x12&\n" +
"\x0eSshFoundConfig\x18\x02 \x01(\tR\x0eSshFoundConfig\"\r\n" +
"\vInitRequest\"\xfb\a\n" +
"\vInitRequest\"\xa2\b\n" +
"\fInitResponse\x12\x1e\n" +
"\n" +
"showFooter\x18\x01 \x01(\bR\n" +
@ -3989,7 +3997,8 @@ const file_olivetin_api_v1_olivetin_proto_rawDesc = "" +
"\n" +
"banner_css\x18\x14 \x01(\tR\tbannerCss\x12)\n" +
"\x10show_diagnostics\x18\x15 \x01(\bR\x0fshowDiagnostics\x12\"\n" +
"\rshow_log_list\x18\x16 \x01(\bR\vshowLogList\"8\n" +
"\rshow_log_list\x18\x16 \x01(\bR\vshowLogList\x12%\n" +
"\x0elogin_required\x18\x17 \x01(\bR\rloginRequired\"8\n" +
"\x0eAdditionalLink\x12\x14\n" +
"\x05title\x18\x01 \x01(\tR\x05title\x12\x10\n" +
"\x03url\x18\x02 \x01(\tR\x03url\"L\n" +

View File

@ -57,6 +57,9 @@ func (u *AuthenticatedUser) parseUsergroupLine(sep string) []string {
} else {
ret = strings.Fields(u.UsergroupLine)
}
log.Debugf("parseUsergroupLine: %v, %v, sep:%v", u.UsergroupLine, ret, sep)
return ret
}

View File

@ -761,9 +761,7 @@ func (api *oliveTinAPI) GetDiagnostics(ctx ctx.Context, req *connect.Request[api
func (api *oliveTinAPI) Init(ctx ctx.Context, req *connect.Request[apiv1.InitRequest]) (*connect.Response[apiv1.InitResponse], error) {
user := acl.UserFromContext(ctx, req, api.cfg)
if err := api.checkDashboardAccess(user); err != nil {
return nil, err
}
loginRequired := user.IsGuest() && api.cfg.AuthRequireGuestsToLogin
res := &apiv1.InitResponse{
ShowFooter: api.cfg.ShowFooter,
@ -788,6 +786,7 @@ func (api *oliveTinAPI) Init(ctx ctx.Context, req *connect.Request[apiv1.InitReq
BannerCss: api.cfg.BannerCSS,
ShowDiagnostics: user.EffectivePolicy.ShowDiagnostics,
ShowLogList: user.EffectivePolicy.ShowLogList,
LoginRequired: loginRequired,
}
return connect.NewResponse(res), nil

View File

@ -429,10 +429,13 @@ func stepACLCheck(req *ExecutionRequest) bool {
func stepParseArgs(req *ExecutionRequest) bool {
ensureArgumentMap(req)
injectSystemArgs(req)
mangleInvalidArgumentValues(req)
if !hasBindingAndAction(req) {
return fail(req, fmt.Errorf("cannot parse arguments: Binding or Action is nil"))
}
mangleInvalidArgumentValues(req)
if hasExec(req) {
return parseExec(req)
}

View File

@ -1,44 +0,0 @@
package httpservers
import (
"net/http"
"strings"
log "github.com/sirupsen/logrus"
// apiv1 "github.com/OliveTin/OliveTin/gen/olivetin/api/v1"
config "github.com/OliveTin/OliveTin/internal/config"
)
func parseHttpHeaderForAuth(cfg *config.Config, req *http.Request) (string, string) {
username, ok := req.Header[cfg.AuthHttpHeaderUsername]
if !ok {
log.Warnf("Config has AuthHttpHeaderUsername set to %v, but it was not found", cfg.AuthHttpHeaderUsername)
return "", ""
}
if cfg.AuthHttpHeaderUserGroup != "" {
usergroup, ok := req.Header[cfg.AuthHttpHeaderUserGroup]
if ok {
log.Debugf("HTTP Header Auth found a username and usergroup")
return username[0], usergroup[0]
} else {
log.Warnf("Config has AuthHttpHeaderUserGroup set to %v, but it was not found", cfg.AuthHttpHeaderUserGroup)
}
}
log.Debugf("HTTP Header Auth found a username, but usergroup is not being used")
return username[0], ""
}
//gocyclo:ignore
func parseJwtHeader(cfg *config.Config, req *http.Request) (string, string) {
// JWTs in the Authorization header are usually prefixed with "Bearer " which is not part of the JWT token.
return parseJwt(cfg, strings.TrimPrefix(req.Header.Get(cfg.AuthJwtHeader), "Bearer "))
}