Introduce ADR #1042

butonic · 2020-12-09T14:12:54Z

We will keep track of Architectual Decision Records using Markdown in /docs/adr.

This PR adds three ADRs which @micbar, @dragotin, @hodyroff, @pmaier1, @IljaN and I were discussing today.

Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>

docs/adr/0002-persist-accounts-using-cs3-storage.md

docs/adr/0003-outsource-user-management.md

Co-authored-by: Alex Unger <6905948+refs@users.noreply.github.com>

docs/adr/0001-introduce-accounts-service.md

docs/adr/0002-persist-accounts-using-cs3-storage.md

docs/adr/0003-outsource-user-management.md

@phil-davis

so embarrassing. Thx @phil-davis Co-authored-by: Phil Davis <phil@jankaritech.com>

docs/adr/0001-introduce-accounts-service.md

docs/adr/0003-outsource-user-management.md

kulmann · 2020-12-10T07:29:19Z

docs/adr/0003-outsource-user-management.md

+- Single binary: admin can manage users, groups and roles using the built in web ui (glauth)
+- External LDAP: OCIS admin needs to use existing tool to manage users
+- Separate OCIS and LDAP admin: OCIS admin relies on the LDAP admin to manage users
+


What about the two LDAP servers scenario, where we use the customers LDAP for internal users and another one (GLAuth?) for guest accounts? Might even make sense to have the web ui for disabling guest users...

GLauth already has a primary and fallback backend. The chain can easily be extended to multiple backends. login and single user properties can even be made in parallel. The only challenge is merging results when searching recipients in multiple ldap servers, but since 1000 results are not helpful for users we can limit the page size to 10 per ldap. Furthermore, multiple ldap servers indicate larger deployments where user lookup via ldap is restricted to exact matches, eg. the full email. In these cases merging becomes trivial again.

We may want to cache these queries anyway in order not flood ldap servers with lookup requests.

We decided to make multiple LDAP servers possible through an additional service which works as a connector to multiple LDAP servers and exposes their user information via the GraphAPI

I would summarize the two comments to "GLAuth abstracts multiple LDAP servers that might exist into one that looks like one to the consumer. A GraphAPI will be provided to interact with GLAuth to manage guest users (for that, at least one of the LDAPs must be read/write)".

Thoughts:

The GLAuth must somehow expose if it allows write ops to create or edit (Guest-) Users or not.

reworked the list

kulmann · 2020-12-10T07:32:41Z

docs/adr/0003-outsource-user-management.md

+- Separate OCIS and LDAP admin: OCIS admin relies on the LDAP admin to manage users
+
+### Resulting technical implications
+- add graphapi to glauth so the ocis web ui can use it to manage users


I thought the web ui should move to GLAuth? Embedding it in oC Web could be done by wrapping it into an oC Web extension (need to check if that's possible. I don't want an iframe solution).

We may not be able to theme the glauth web ui as we like it. Also the tight integration we have in the current implementation is awesome / slick. We need to embed glauth anyway. The first step would be to treat the current accounts service as the drop in ldap server with a web ui to manage users. It would mean merging ocis-accounts and ocis-glauth.
The next step would be to add the graph api to glauth. then we can build a web ui that uses glauth to manage accounts.

This accounts-management service would be a web ui to manage accounts via the graph api. It could be part of glauth or we use an embedded, tightly integrated version for ocis-web that follows our theme.

The final step would be a graphapi implementation that is backed by a real ldap server. I'd put that into glauth because it already has several backends. Having a graphapi <-> ldap proxy would fit GLauth I think.

But in order to have a truly embeddable web ui I think glauth shoul start with the graph api. It can then have a web ui or projects that want to embed it can build the ui in the way they want. The kopano guys @fbartels @longsleep might want to use react to embed it into konnectd.

added as a bullet point, because I agree: keep the ui in GLAuth

kulmann · 2020-12-10T07:36:00Z

docs/adr/0003-outsource-user-management.md

+
+### Resulting technical implications
+- add graphapi to glauth so the ocis web ui can use it to manage users
+- make graphapi service to directly talk to an LDAP server so our web ui can use it


I don't get this one 😅 we don't want to bypass LDAP admins. Thought that the web ui for user management is only supposed to be used in case we have GLAuth as LDAP replacement.

The Basic idea is the following: We don't need write access via the LDAP protocol when a IDM would have a second interaction point via the GraphAPI.

We can imagine quite a lot of scenarios, where e.g. the provisoining of guest users in the IDM would happen via a public http api endpoint.

ah but we learned that for small scale deployments some admins actually prefer a simple ui ... 🤷
If the web ui is self contained, having it in glauth should be fine. I don't want to build the next keycloak ...

I thought that even the super-duper LDAP is covered by a thin layer of GLAuth?

Why would a customer give us write access via GraphAPI when they don't want to give us write access via LDAP? 🤔 I thought about the web ui as an option to provide easy to use read/write access for the GLAuth tiny setup use case. As in the 5 people company that doesn't have an actual LDAP, the family who want to handle their photos and the like. Why would we want to build an accounts web ui for Siemens? They have their own tools for user management.

GLAuth cannot write to an ldap server, it can only write to other backends.

Reworded the list

docs/adr/0003-outsource-user-management.md

Co-authored-by: Benedikt Kulmann <benedikt@kulmann.biz>

sonarqubecloud · 2020-12-10T07:56:35Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

micbar

@kulmann @butonic I tried to clarify the Graph API ideas.

It would help to have a diagram. How do diagrams fit into that ADR format?

butonic · 2020-12-10T08:03:32Z

it is madr ... so markdown ... we should be able to embed .drawio.svg files ...

butonic · 2020-12-10T08:08:06Z

@micbar maybe we can manage the docs generation with our normal docs pipeline?

micbar · 2020-12-10T08:25:28Z

@butonic yes. First merge it. Then hook it up to hugo.

docs/adr/0003-outsource-user-management.md

dragotin

It would be awesome to have a little diagram attached...

docs/adr/0002-persist-accounts-using-cs3-storage.md

docs/adr/0003-outsource-user-management.md

dragotin · 2020-12-10T13:33:42Z

docs/adr/0003-outsource-user-management.md

+- Single binary: admin can manage users, groups and roles using the built in web ui (glauth)
+- External LDAP: OCIS admin needs to use existing tool to manage users
+- Separate OCIS and LDAP admin: OCIS admin relies on the LDAP admin to manage users
+


I would summarize the two comments to "GLAuth abstracts multiple LDAP servers that might exist into one that looks like one to the consumer. A GraphAPI will be provided to interact with GLAuth to manage guest users (for that, at least one of the LDAPs must be read/write)".

Thoughts:

The GLAuth must somehow expose if it allows write ops to create or edit (Guest-) Users or not.

dragotin · 2020-12-10T13:38:08Z

docs/adr/0003-outsource-user-management.md

+
+### Resulting technical implications
+- add graphapi to glauth so the ocis web ui can use it to manage users
+- make graphapi service to directly talk to an LDAP server so our web ui can use it


I thought that even the super-duper LDAP is covered by a thin layer of GLAuth?

Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>

butonic · 2021-02-09T21:46:57Z

@micbar @dragotin @pmaier1 @kulmann I finally incorporated your feedback. No diagrams though. Show me your thumbs!

kulmann

👍
(comments are just about some typos etc)

docs/adr/0001-introduce-accounts-service.md

docs/adr/0003-external-user-management.md

pmaier1

<3

Co-authored-by: Benedikt Kulmann <benedikt@kulmann.biz>

refs

👍

fschade

👍

Merge: 182f895 1ddfb7c Author: Klaas Freitag <kfreitag@owncloud.com> Date: Wed Feb 10 18:05:51 2021 +0100 Merge pull request #1042 from owncloud/introduce-adr Introduce ADR

Introduce ADR

7fa0bf1

Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>

refs suggested changes Dec 9, 2020

View reviewed changes

Apply suggestions from code review

cca8cb4

Co-authored-by: Alex Unger <6905948+refs@users.noreply.github.com>

refs approved these changes Dec 9, 2020

View reviewed changes

phil-davis reviewed Dec 9, 2020

View reviewed changes

butonic and others added 2 commits December 9, 2020 15:54

correct github nicks

972ef91

Oh my god? all the typos ...

da525ae

so embarrassing. Thx @phil-davis Co-authored-by: Phil Davis <phil@jankaritech.com>

butonic requested review from C0rby, kulmann, dragotin, hodyroff, IljaN, micbar and pmaier1 December 10, 2020 06:59

kulmann suggested changes Dec 10, 2020

View reviewed changes

Apply suggestions from code review

f6ac7b7

Co-authored-by: Benedikt Kulmann <benedikt@kulmann.biz>

micbar reviewed Dec 10, 2020

View reviewed changes

butonic requested review from fschade and wkloucek December 10, 2020 08:28

pmaier1 reviewed Dec 10, 2020

View reviewed changes

docs/adr/0003-outsource-user-management.md Outdated Show resolved Hide resolved

dragotin reviewed Dec 10, 2020

View reviewed changes

refs self-requested a review December 10, 2020 13:45

This was referenced Jan 22, 2021

Where to put feature requests, Architecture decisions, high level goals or fuzzy ideas? #1391

Closed

Clarify single machine vs. HA / cloud setup #1392

Closed

incorporate feedback

e864db8

Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>

IljaN approved these changes Feb 9, 2021

View reviewed changes

kulmann approved these changes Feb 10, 2021

View reviewed changes

pmaier1 approved these changes Feb 10, 2021

View reviewed changes

Apply suggestions from code review

1ddfb7c

Co-authored-by: Benedikt Kulmann <benedikt@kulmann.biz>

refs approved these changes Feb 10, 2021

View reviewed changes

wkloucek approved these changes Feb 10, 2021

View reviewed changes

micbar approved these changes Feb 10, 2021

View reviewed changes

butonic requested a review from dragotin February 10, 2021 12:18

C0rby approved these changes Feb 10, 2021

View reviewed changes

fschade approved these changes Feb 10, 2021

View reviewed changes

dragotin approved these changes Feb 10, 2021

View reviewed changes

dragotin merged commit 6ef5794 into master Feb 10, 2021

delete-merged-branch bot deleted the introduce-adr branch February 10, 2021 17:05

ownclouders pushed a commit that referenced this pull request Feb 10, 2021

commit 6ef5794

79001ef

Merge: 182f895 1ddfb7c Author: Klaas Freitag <kfreitag@owncloud.com> Date: Wed Feb 10 18:05:51 2021 +0100 Merge pull request #1042 from owncloud/introduce-adr Introduce ADR

micbar mentioned this pull request Feb 17, 2021

Release 1.2.0 Tech Preview #1654

Closed

16 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce ADR #1042

Introduce ADR #1042

butonic commented Dec 9, 2020 •

edited

Loading

kulmann Dec 10, 2020

butonic Dec 10, 2020

micbar Dec 10, 2020

dragotin Dec 10, 2020

butonic Feb 9, 2021

kulmann Dec 10, 2020

butonic Dec 10, 2020

butonic Feb 9, 2021

kulmann Dec 10, 2020

micbar Dec 10, 2020

butonic Dec 10, 2020

dragotin Dec 10, 2020

kulmann Dec 10, 2020

butonic Feb 9, 2021

sonarqubecloud bot commented Dec 10, 2020

micbar left a comment

butonic commented Dec 10, 2020

butonic commented Dec 10, 2020

micbar commented Dec 10, 2020

dragotin left a comment

dragotin Dec 10, 2020

dragotin Dec 10, 2020

butonic commented Feb 9, 2021

kulmann left a comment

pmaier1 left a comment

refs left a comment

fschade left a comment

Introduce ADR #1042

Introduce ADR #1042

Conversation

butonic commented Dec 9, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarqubecloud bot commented Dec 10, 2020

micbar left a comment

Choose a reason for hiding this comment

butonic commented Dec 10, 2020

butonic commented Dec 10, 2020

micbar commented Dec 10, 2020

dragotin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

butonic commented Feb 9, 2021

kulmann left a comment

Choose a reason for hiding this comment

pmaier1 left a comment

Choose a reason for hiding this comment

refs left a comment

Choose a reason for hiding this comment

fschade left a comment

Choose a reason for hiding this comment

butonic commented Dec 9, 2020 •

edited

Loading