feat(idp): support login page background configuration #7900

brandon1024 · 2023-12-07T00:09:57Z

Description

This revision introduces a new environment variable IDP_LOGIN_BACKGROUND_URL that overrides the default background image of the IDP login page when present.

Related Issue

Fixes [enhancement] appearance: ability to upload login page background image #7674

Motivation and Context

First, I want to confess that I'm by no means a frontend developer. I'll happily accept any feedback :-)

One of the main motivations of this change is to get closer to improving consistency across the services that I host in my lab. It's possible to configure the PhotoPrism background image, and I wanted a similar look and feel with owncloud.

My original idea was to allow the user to upload or configure an image, but that's a fair bit of work and it doesn't need to be that complicated. Simply updating the stylesheet with an externally configurable address is good enough ™️

Discussed in #7674 was the eventual migration to another provider like Authelia, and I understand that this feature will become irrelevant when that migration happens. I think this is acceptable, and I think this small feature provides good value in the meantime 🙂

How Has This Been Tested?

Tested with and without IDP_LOGIN_BACKGROUND_URL:

$ docker build -t owncloud/ocis:dev .
$ docker run --rm -ti -p 9200:9200 --env IDP_LOGIN_BACKGROUND_URL=${MY_RESOURCE} --entrypoint sh owncloud/ocis:dev -c 'ocis init || true; ocis server'

Then, in the browser navigate to locahost:9200.

Screenshots (if appropriate):

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)
Technical debt
Tests only (no source changes)

Checklist:

Code changes
Unit tests added (there are none!)
Acceptance tests added
Documentation ticket raised:

update-docs · 2023-12-07T00:10:02Z

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

brandon1024 · 2023-12-07T00:15:50Z

Edit: fixed in #7915

Note for reviewers, I wasn't able to build the project without updating the Go version in the dockerfile. Some modules were using a newer Go version.

Here's what I was seeing:

$ docker build -t owncloud/ocis:dev .

[... truncated for brevity]

 => ERROR [build 4/4] RUN make ci-go-generate build                                                                                                                                                                                                                          1.5s
------
 > [build 4/4] RUN make ci-go-generate build:
0.459 make: Nothing to be done for 'ci-go-generate'.
0.473 go build -v -tags 'disable_crypt' -ldflags '-X google.golang.org/protobuf/reflect/protoregistry.conflictPolicy=warn -s -w -X "github.com/owncloud/ocis/v2/ocis-pkg/version.String=200f40206b" -X "github.com/owncloud/ocis/v2/ocis-pkg/version.Tag=" -X "github.com/owncloud
/ocis/v2/ocis-pkg/version.Date=20231207"' -o bin/ocis ./cmd/ocis
1.153 ../services/graph/pkg/unifiedrole/unifiedrole.go:4:2: package cmp is not in GOROOT (/usr/local/go/src/cmp)
1.153 note: imported by a module that requires go 1.21
1.153 ../services/graph/pkg/errorcode/cs3.go:4:2: package slices is not in GOROOT (/usr/local/go/src/slices)
1.153 note: imported by a module that requires go 1.21
1.159 make: *** [../.make/go.mk:102: bin/ocis] Error 1
------
Dockerfile:29
--------------------
  27 |     
  28 |     WORKDIR /ocis/ocis
  29 | >>> RUN make ci-go-generate build
  30 |     
  31 |     FROM alpine:3.18
--------------------
ERROR: failed to solve: process "/bin/sh -c make ci-go-generate build" did not complete successfully: exit code: 2

Here was the fix:

diff --git a/Dockerfile b/Dockerfile
index 48ac9f23a0..24dc73d61d 100644
--- a/Dockerfile
+++ b/Dockerfile
@@ -21,7 +21,7 @@ COPY ./ /ocis/
 WORKDIR /ocis/ocis
 RUN make ci-node-generate
 
-FROM owncloudci/golang:1.20 as build
+FROM owncloudci/golang:1.21 as build
 
 COPY --from=generate /ocis /ocis

services/idp/public/index.html

AlexAndBear · 2023-12-11T20:13:19Z

services/idp/src/index.tsx

@@ -9,11 +9,18 @@ import store from './store';

 import './app.css';

+const root = document.getElementById('root')
+
+// if a custome background image has been configured, use it


Suggested change

// if a custome background image has been configured, use it

// if a custom background image has been configured, make use of it

Oops 😅 Will include in the next patch

AlexAndBear · 2023-12-11T20:15:34Z

services/idp/src/index.tsx

 ReactDOM.render(
  <React.StrictMode>
    <Provider store={store as any}>
-      <App/>
+      <div className='oc-login-bg' style={bgImg ? { backgroundImage: `url(${bgImg})` } : {}}>


Is it possible to have the attributes on the App or even deeper (slot or something), I'd like to avoid adding an extra wrapping div

I was hoping to go that route, but I had some trouble getting the background to render when I added this directly on the App or children. I'm don't know enough about Material UI to know why that isn't working :/

I made a few adjustments though, I moved the div down to App and pass the image url as a prop. I think it's a bit cleaner, keeping the styling contained in App and out of the root component.

services/idp/pkg/config/config.go

changelog/unreleased/enhancement-idp-config-background-img.md

AlexAndBear · 2023-12-12T03:41:22Z

services/idp/src/App.jsx

@@ -13,14 +14,23 @@ const LazyMain = lazy(() => import(/* webpackChunkName: "identifier-main" */ './

 console.info(`Kopano Identifier build version: ${version.build}`); // eslint-disable-line no-console

-const App = (): ReactElement => {
+const App = ({ bgImg }) => {


Could you check the type again? Might be ReactNode ?

Definitely :-)

This revision introduces a new environment variable `IDP_LOGIN_BACKGROUND_URL` that overrides the default background image of the IDP login page when present.

brandon1024 · 2024-01-05T15:28:24Z

Rebased onto the tip of master.

@kulmann @janackermann this should be ready for re-review :-)

sonarcloud · 2024-01-05T15:40:48Z

Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

kulmann

😍

feat(idp): support login page background configuration

brandon1024 mentioned this pull request Dec 7, 2023

Dockerfile uses go 1.20, but need go 1.21 #7911

Closed

brandon1024 force-pushed the idp-bg-config branch from 200f402 to 1c9d108 Compare December 10, 2023 21:54

AlexAndBear reviewed Dec 11, 2023

View reviewed changes

services/idp/public/index.html Show resolved Hide resolved

AlexAndBear reviewed Dec 11, 2023

View reviewed changes

kulmann reviewed Dec 11, 2023

View reviewed changes

services/idp/pkg/config/config.go Outdated Show resolved Hide resolved

changelog/unreleased/enhancement-idp-config-background-img.md Show resolved Hide resolved

brandon1024 force-pushed the idp-bg-config branch from 1c9d108 to 2e3b6e7 Compare December 12, 2023 03:00

AlexAndBear reviewed Dec 12, 2023

View reviewed changes

brandon1024 force-pushed the idp-bg-config branch from 2e3b6e7 to 88a4561 Compare December 12, 2023 11:56

brandon1024 requested review from AlexAndBear and kulmann December 14, 2023 12:04

brandon1024 added 2 commits January 5, 2024 11:27

feat(idp): support login page background configuration

9bbd993

This revision introduces a new environment variable `IDP_LOGIN_BACKGROUND_URL` that overrides the default background image of the IDP login page when present.

chore(docs): add changelog item

fbb4f9b

brandon1024 force-pushed the idp-bg-config branch from 88a4561 to fbb4f9b Compare January 5, 2024 15:27

kulmann approved these changes Jan 31, 2024

View reviewed changes

kulmann merged commit 7f2d2d2 into owncloud:master Jan 31, 2024
3 checks passed

AlexAndBear approved these changes Jan 31, 2024

View reviewed changes

ownclouders pushed a commit that referenced this pull request Jan 31, 2024

Merge pull request #7900 from brandon1024/idp-bg-config

bc02773

feat(idp): support login page background configuration

brandon1024 deleted the idp-bg-config branch January 31, 2024 15:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(idp): support login page background configuration #7900

feat(idp): support login page background configuration #7900

brandon1024 commented Dec 7, 2023

update-docs bot commented Dec 7, 2023

brandon1024 commented Dec 7, 2023 •

edited

AlexAndBear Dec 11, 2023

brandon1024 Dec 12, 2023

AlexAndBear Dec 11, 2023

brandon1024 Dec 12, 2023

AlexAndBear Dec 12, 2023

brandon1024 Dec 12, 2023

brandon1024 commented Jan 5, 2024

sonarcloud bot commented Jan 5, 2024

kulmann left a comment

	// if a custome background image has been configured, use it
	// if a custom background image has been configured, make use of it

feat(idp): support login page background configuration #7900

feat(idp): support login page background configuration #7900

Conversation

brandon1024 commented Dec 7, 2023

Description

Related Issue

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

Checklist:

update-docs bot commented Dec 7, 2023

brandon1024 commented Dec 7, 2023 • edited

AlexAndBear Dec 11, 2023

Choose a reason for hiding this comment

brandon1024 Dec 12, 2023

Choose a reason for hiding this comment

AlexAndBear Dec 11, 2023

Choose a reason for hiding this comment

brandon1024 Dec 12, 2023

Choose a reason for hiding this comment

AlexAndBear Dec 12, 2023

Choose a reason for hiding this comment

brandon1024 Dec 12, 2023

Choose a reason for hiding this comment

brandon1024 commented Jan 5, 2024

sonarcloud bot commented Jan 5, 2024

Quality Gate passed

kulmann left a comment

Choose a reason for hiding this comment

brandon1024 commented Dec 7, 2023 •

edited