Skip to content

Commit

Permalink
website/admin: fix more issues with the profile pages
Browse files Browse the repository at this point in the history
storage/mariadb: fix UserStorage.Get looking up from the djs table, not the users table

website/middleware: add RequestURL to shared input
  • Loading branch information
Wessie committed Apr 25, 2024
1 parent 99a28d2 commit afd48a3
Show file tree
Hide file tree
Showing 4 changed files with 176 additions and 38 deletions.
32 changes: 16 additions & 16 deletions storage/mariadb/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -413,27 +413,27 @@ func (us UserStorage) All() ([]radio.User, error) {
users.created_at AS created_at,
IFNULL(users.user, '') AS username,
(SELECT group_concat(permission) FROM permissions WHERE user_id=users.id) AS userpermissions,
djs.id AS 'dj.id',
djs.regex AS 'dj.regex',
djs.djname AS 'dj.name',
djs.djtext AS 'dj.text',
djs.djimage AS 'dj.image',
djs.visible AS 'dj.visible',
djs.priority AS 'dj.priority',
djs.role AS 'dj.role',
djs.css AS 'dj.css',
djs.djcolor AS 'dj.color',
IFNULL(djs.id, 0) AS 'dj.id',
IFNULL(djs.regex, '') AS 'dj.regex',
IFNULL(djs.djname, '') AS 'dj.name',
IFNULL(djs.djtext, '') AS 'dj.text',
IFNULL(djs.djimage, '') AS 'dj.image',
IFNULL(djs.visible, 0) AS 'dj.visible',
IFNULL(djs.priority, 0) AS 'dj.priority',
IFNULL(djs.role, '') AS 'dj.role',
IFNULL(djs.css, '') AS 'dj.css',
IFNULL(djs.djcolor, '') AS 'dj.color',
IFNULL(themes.id, 0) AS 'dj.theme.id',
IFNULL(themes.name, 'default') AS 'dj.theme.name',
IFNULL(themes.display_name, 'default') AS 'dj.theme.displayname',
IFNULL(themes.author, 'unknown') AS 'dj.theme.author'
FROM
djs
JOIN
users ON djs.id = users.djid
users
LEFT JOIN
djs ON djs.id = users.djid
LEFT JOIN
themes ON djs.theme_id = themes.id;
`
Expand Down
62 changes: 52 additions & 10 deletions website/admin/profiles.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,29 @@ func (ProfileForm) TemplateName() string {
return "form_profile"
}

const profileFormAction = "/admin/profile"

func (ProfileForm) FormAction() template.HTMLAttr {
return profileFormAction
}

func (ProfileForm) FormType() template.HTMLAttr {
return `enctype="multipart/form-data"`
}

func (f *ProfileForm) AddDJProfileURL() template.URL {
uri, err := url.Parse(profileFormAction)
if err != nil {
return ""
}

query := uri.Query()
query.Set("username", f.Username)
query.Set("new", profileNewDJ)
uri.RawQuery = query.Encode()
return template.URL(uri.String())
}

// GetProfile is mounted under /admin/profile and shows the currently logged
// in users profile
func (s *State) GetProfile(w http.ResponseWriter, r *http.Request) {
Expand Down Expand Up @@ -275,6 +298,10 @@ func (s *State) postNewProfileUser(r *http.Request, username string) (*ProfileFo
return nil, errors.E(op, err)
}

// NewProfileForm will have emptied our UserPermissions because the new user
// form doesn't send any, so just overwrite it here
form.UserPermissions = user.UserPermissions

// generate a password for the user
newPassword, err := postProfilePassword(form.PasswordChangeForm, true)
if err != nil {
Expand Down Expand Up @@ -316,14 +343,15 @@ func (s *State) postNewProfileDJ(r *http.Request, username string) (*ProfileForm
if err != nil {
return form, errors.E(op, err)
}
form.User.DJ.ID = djid
dj.ID = djid // apply the id
form.User.DJ = dj // then add it to the form we're returning
return form, nil
}

func (s *State) postNewProfile(r *http.Request) (*ProfileForm, error) {
const op errors.Op = "website/admin.postNewProfile"

newMode := r.FormValue("new") // what we're making a new thing off
newMode := r.FormValue("new") // what we're making a new thing of
username := r.FormValue("username") // the user we're making a new thing for

switch newMode {
Expand Down Expand Up @@ -447,7 +475,7 @@ func NewProfileForm(user radio.User, r *http.Request) (*ProfileForm, error) {

// initial check to see if we're actually editing the expected user
if values.Get("username") != user.Username {
return nil, errors.E(op, errors.AccessDenied)
return nil, errors.E(op, errors.AccessDenied, "username does not match")
}

form := newProfileForm(user, r)
Expand All @@ -473,16 +501,30 @@ func newProfileForm(user radio.User, r *http.Request) ProfileForm {
}

func (pf *ProfileForm) Update(form url.Values) {
pf.Username = form.Get("username")
pf.IP = form.Get("ip")
pf.Email = form.Get("email")
pf.DJ.Visible = form.Get("dj.visible") != ""
pf.DJ.Name = form.Get("dj.name")
if form.Has("username") {
pf.Username = form.Get("username")
}
if form.Has("ip") {
pf.IP = form.Get("ip")
}
if form.Has("email") {
pf.Email = form.Get("email")
}
if form.Has("dj.visible") {
pf.DJ.Visible = form.Get("dj.visible") != ""
}
if form.Has("dj.name") {
pf.DJ.Name = form.Get("dj.name")
}
if prio, err := strconv.Atoi(form.Get("dj.priority")); err == nil {
pf.DJ.Priority = prio
}
pf.DJ.Regex = form.Get("dj.regex")
pf.DJ.Theme.Name = form.Get("dj.theme.name")
if form.Has("dj.regex") {
pf.DJ.Regex = form.Get("dj.regex")
}
if form.Has("dj.theme.name") {
pf.DJ.Theme.Name = form.Get("dj.theme.name")
}

// password handling
pf.PasswordChangeForm.Current = form.Get("password.current")
Expand Down
94 changes: 94 additions & 0 deletions website/admin/profiles_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@ package admin

import (
"context"
"maps"
"net/http"
"net/http/httptest"
"reflect"
"strings"
"testing"

Expand All @@ -12,6 +14,10 @@ import (
"github.com/R-a-dio/valkyrie/errors"
"github.com/R-a-dio/valkyrie/mocks"
"github.com/R-a-dio/valkyrie/website/middleware"
"github.com/leanovate/gopter"
"github.com/leanovate/gopter/arbitrary"
"github.com/leanovate/gopter/gen"
"github.com/leanovate/gopter/prop"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
Expand All @@ -37,6 +43,9 @@ type profileTest struct {
CreateRet radio.UserID
CreateErr error

CreateDJRet radio.DJID
CreateDJErr error

GetRet radio.User
GetErr error

Expand Down Expand Up @@ -80,6 +89,9 @@ var profileTests = []profileTest{
Form: ProfileForm{
User: radio.User{
Username: "newuser",
UserPermissions: radio.UserPermissions{
radio.PermActive: struct{}{},
},
},
PasswordChangeForm: ProfilePasswordChangeForm{
New: "hackme",
Expand Down Expand Up @@ -107,6 +119,26 @@ var profileTests = []profileTest{
ExpectedForm: nil,
Error: errors.E(errors.AccessDenied),
},
{
Name: "NewDJProfileCreation",
Path: "/admin/profile?new=" + profileNewDJ,
User: *adminUser,
Form: ProfileForm{
User: *profileTestUser,
},
ExpectedForm: &ProfileForm{
User: mutateUser(*profileTestUser, func(u radio.User) radio.User {
u.DJ = newProfileDJ(u.Username)
u.DJ.ID = 70 // should be same as CreateDJRet
return u
}),
},
TxFunc: mocks.CommitTx,
GetRet: *profileTestUser,
GetErr: nil,
CreateDJRet: 70,
CreateDJErr: nil,
},
{
// permissions update executed by an admin, should work
Name: "UpdatePermissionsAsAdmin",
Expand All @@ -124,6 +156,7 @@ var profileTests = []profileTest{
ExpectedForm: profileSameAsInput,
TxFunc: mocks.CommitTx,
GetRet: *profileTestUser,
GetErr: nil,
},
{
// permissions update by the user themselves, should not work,
Expand All @@ -147,6 +180,7 @@ var profileTests = []profileTest{
},
TxFunc: mocks.CommitTx,
GetRet: *profileTestUser,
GetErr: nil,
},
{
// users should be able to update their own password assuming they know
Expand All @@ -167,6 +201,7 @@ var profileTests = []profileTest{
ExpectedPassword: "donthackme",
TxFunc: mocks.CommitTx,
GetRet: *profileTestUser,
GetErr: nil,
},
{
// should only be able to change passwords if New and Repeated match
Expand All @@ -186,6 +221,7 @@ var profileTests = []profileTest{
ExpectedPassword: "hackme",
TxFunc: mocks.CommitTx,
GetRet: *profileTestUser,
GetErr: nil,
Error: errors.E(errors.InvalidForm),
},
{
Expand All @@ -206,6 +242,7 @@ var profileTests = []profileTest{
ExpectedPassword: "hackme",
TxFunc: mocks.CommitTx,
GetRet: *profileTestUser,
GetErr: nil,
Error: errors.E(errors.AccessDenied),
},
{
Expand All @@ -225,6 +262,7 @@ var profileTests = []profileTest{
ExpectedPassword: "hackme",
TxFunc: mocks.CommitTx,
GetRet: *profileTestUser,
GetErr: nil,
Error: errors.E(errors.InvalidForm),
},
{
Expand All @@ -244,6 +282,7 @@ var profileTests = []profileTest{
ExpectedPassword: "donthackme",
TxFunc: mocks.CommitTx,
GetRet: *profileTestUser,
GetErr: nil,
},
}

Expand Down Expand Up @@ -293,6 +332,10 @@ func TestPostProfile(t *testing.T) {
assert.Equal(t, test.ExpectedForm.Username, user.Username)
return test.UpdateRet, test.UpdateErr
},
CreateDJFunc: func(user radio.User, dJ radio.DJ) (radio.DJID, error) {
assert.Equal(t, test.ExpectedForm.Username, user.Username)
return test.CreateDJRet, test.CreateDJErr
},
}
}

Expand Down Expand Up @@ -350,4 +393,55 @@ func checkForm(t *testing.T, test profileTest, got *ProfileForm) {

assert.NoError(t, got.User.ComparePassword(test.ExpectedPassword))
assert.Equal(t, expected.Username, got.Username)
assert.Equal(t, expected.UserPermissions, got.UserPermissions)
assert.Equal(t, expected.DJ, got.DJ)
}

func TestProfileFormRoundTrip(t *testing.T) {
a := arbitrary.DefaultArbitraries()
p := gopter.NewProperties(nil)

profileUserGen := gen.Struct(reflect.TypeOf(radio.User{}), map[string]gopter.Gen{
"Username": gen.AnyString(),
"Email": gen.AnyString(),
"IP": gen.AnyString(),
"UserPermissions": genForType[radio.UserPermissions](a),
"DJ": gen.Struct(reflect.TypeOf(radio.DJ{}), map[string]gopter.Gen{
"ID": genForType[radio.DJID](a).SuchThat(func(v radio.DJID) bool {
return v != 0
}),
"Visible": gen.Bool(),
"Name": gen.AnyString(),
"Priority": gen.Int(),
"Regex": gen.AnyString(),
}),
})

p.Property("form should roundtrip", prop.ForAll(
func(u radio.User) bool {
in := ProfileForm{
User: u,
}
var out ProfileForm

out.Update(in.ToValues())
// null the djid, since we don't actually roundtrip it but it is required
// by the ToValues to be set
in.DJ.ID = 0

return in.Username == out.Username &&
in.Email == out.Email &&
in.IP == out.IP &&
in.DJ == out.DJ &&
in.PasswordChangeForm.Current == out.PasswordChangeForm.Current &&
in.PasswordChangeForm.New == out.PasswordChangeForm.New &&
in.PasswordChangeForm.Repeated == out.PasswordChangeForm.Repeated &&
maps.Equal(in.UserPermissions, out.newPermissions)
}, profileUserGen,
))
p.TestingRun(t, gopter.ConsoleReporter(false))
}

func genForType[T any](a *arbitrary.Arbitraries) gopter.Gen {
return a.GenForType(reflect.TypeFor[T]())
}
26 changes: 14 additions & 12 deletions website/middleware/input.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,13 @@ func InputMiddleware(cfg config.Config, status *util.Value[radio.Status]) func(h

user := UserFromContext(ctx)
input := Input{
Now: time.Now(),
IsHTMX: util.IsHTMX(r),
IsUser: user != nil,
User: user,
StreamURL: PublicStreamURL,
Status: status.Latest(),
Now: time.Now(),
IsHTMX: util.IsHTMX(r),
IsUser: user != nil,
User: user,
StreamURL: PublicStreamURL,
RequestURL: template.URL(r.URL.String()),
Status: status.Latest(),
}

ctx = context.WithValue(ctx, inputKey{}, input)
Expand All @@ -53,12 +54,13 @@ func InputFromContext(ctx context.Context) Input {

// Input is a bunch of data that should be accessable to the base template
type Input struct {
Now time.Time
IsUser bool
IsHTMX bool
User *radio.User
Status radio.Status
StreamURL template.URL
Now time.Time
IsUser bool
IsHTMX bool
User *radio.User
Status radio.Status
StreamURL template.URL
RequestURL template.URL
}

func (Input) TemplateName() string {
Expand Down

0 comments on commit afd48a3

Please sign in to comment.