Skip to content

Groupware#1994

Open
pbleser-oc wants to merge 238 commits into
mainfrom
groupware
Open

Groupware#1994
pbleser-oc wants to merge 238 commits into
mainfrom
groupware

Conversation

@pbleser-oc
Copy link
Copy Markdown
Member

@pbleser-oc pbleser-oc commented Dec 9, 2025

Description

Ongoing implementation of the Groupware backend service, which initially happened on its own branch but should now occur on the main branch.

Changes are almost exclusively on packages that are distinct to the Groupware backend, namely

  • pkg/jmap, pkg/jscalendar and pkg/jscontact: contain the implementation of the JMAP protocol and data models for Core, Mail, Contacts, Calendars, Blobs, ...
  • services/groupware: contains the Groupware backend service which is currently configured to not be started by default, and must be explicitly included in the START_ADDITIONAL_SERVICES property

Changes to common areas include:

  • devtools/deployments/opencloud_full: addition of a container of the Stalwart mail server which is used for Groupware functionality
  • pkg/structs: add more helper functions that are then used in pkg/jmap and services/groupware

Note that it is not fully functional yet and is going to be under continued and ongoing development along with its UI counterparts.

Specifically, the following aspects are lacking and only implemented in a skeletal way as a proof of concept:

  • API documentation: originally developed using go-swagger and redocly-cli, but which is having severe limitations and might be superseded by an ongoing implementation of a custom source code parser which renders OpenAPI
  • unit tests: very little coverage if at all, which is mostly due to most of the implementation of the Groupware backend doing relatively little logic and is mostly implementing the JMAP model
  • integration tests: some coverage exists that performs tests against a Stalwart container using testcontainers, but will be made more exhaustive
  • k6 black box tests: none implemented yet
  • system documentation: none implemented yet, too early to do so

The Groupware backend is not meant to be used productively yet.

@pbleser-oc pbleser-oc self-assigned this Dec 9, 2025
@pbleser-oc pbleser-oc requested a review from butonic December 10, 2025 08:32
@pbleser-oc pbleser-oc marked this pull request as ready for review December 10, 2025 08:32
Comment thread services/auth-api/pkg/service/http/v0/service.go Outdated
Comment thread services/groupware/DEVELOPER.md Outdated
Comment thread services/groupware/DEVELOPER.md Outdated
curl -ks -D- -X POST \
"https://keycloak.opencloud.test/realms/openCloud/protocol/openid-connect/token" \
-d username=alan -d password=demo -d grant_type=password \
-d client_id=groupware -d scope=openid
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when working from top to bottom this fails. using a client_id=web works ... so I need to add a client id for the goupware somewhere ...

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did add a JSON file devtools/deployments/opencloud_full/config/keycloak/clients/groupware.json, expecting it to be automatically picked up, but that might require re-creating the Keycloak container... ?
In any case, that was a beginner's mistake, there is really no reason to have a distinct client ID for the groupware, as the Web UI is going to use web instead.

Removed that config file and updated the documentation accordingly to use web instead, in cfbbe02

to which one should receive the following response:

```java
A OK [CAPABILITY IMAP4rev2 ...] Authentication successful
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hm, I need to dig into:

stalwart-1            | 2025-12-10T16:07:05Z TRACE Raw IMAP input received (imap.raw-input) listenerId = "imaptls", localPort = 993, remoteIp = 172.39.0.1, remotePort = 45034, size = 19, contents = "A LOGIN alan demo\r\n"
stalwart-1            | 2025-12-10T16:07:05Z ERROR LDAP error (store.ldap-error) listenerId = "imaptls", localPort = 993, remoteIp = 172.39.0.1, remotePort = 45034, reason = "I/O error: Connection refused (os error 111)", causedBy = "crates/directory/src/core/dispatch.rs:25", id = "A"

Comment thread devtools/deployments/opencloud_full/.env
Comment thread services/groupware/DEVELOPER.md
Comment thread services/groupware/pkg/groupware/api.go
Comment thread pkg/jmap/api.go
@@ -0,0 +1,49 @@
package jmap
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should drop the jmap_ prefix from all files in this package

AUTH_BASIC_LDAP_BIND_PASSWORD: "admin"
USERS_LDAP_BIND_PASSWORD: "admin"
GROUPS_LDAP_BIND_PASSWORD: "admin"
IDM_LDAPS_ADDR: 0.0.0.0:9235
Copy link
Copy Markdown
Member

@butonic butonic Dec 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I configured everything on .compose.test to prevent collisions with my .opencloud.test deployment. The stalwart URL then needs to be set for the groupware:

Suggested change
IDM_LDAPS_ADDR: 0.0.0.0:9235
IDM_LDAPS_ADDR: 0.0.0.0:9235
GROUPWARE_JMAP_BASE_URL: https://${STALWART_DOMAIN:-stalwart.opencloud.test}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh and we need to set FRONTEND_GROUPWARE_ENABLED: "true" and enable the mail app in the web config when stalwart is enabled ... but that connot be configured with a simple env var ... 😞 we need to replace the config file then ...

Copy link
Copy Markdown
Member

@butonic butonic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I still don't have a menu entry for mail in the web UI even though the config.json contains the mail app. I assume that is a problem with my setup/config. in any case, I can see the mail UI when navigation to {$OC_URL}/mail manually. \o/

I did run into some minor issues when following the docs. If you could address them (and enlighten me on how to get menu to show the groupware apps) I'm happy to merge this. I mostly want others to be able follow the DEVELOPMENT.md and have a working setup. Kudos for that, btw.

The groupware service itself follows our ... boilerplate ... service code ... and implements the JMAP handling. I assume that will have to evolve, but we can merge it and iterate.

Tip

the web repo has a compose file with all the apps enabled. That gave me the final hint to get the menu entries

 * add Groupware APIs for creating and deleting addressbooks

 * add Groupware APIs for creating and deleting calendars

 * add JMAP APIs for creating and deleting addressbooks, calendars

 * add JMAP APIs to retrieve Principals

 * fix API tagging

 * move addressbook JMAP APIs into its own file

 * move addressbook Groupware APIs into its own file
…hods and more intelligence to the various request and response types to improve the use of template functions, reducing the risks of typos and copy/paste mistakes
 * move ContactCard from jscontact to jmap, as it is actually a JMAP
   specification item, but also was causing too many issues with
   circular references from jscontact -> jmap

 * introduce Foo, Idable, GetRequest, GetResponse, etc... types and
   generics

 * first attempt at a Foo factory type for Mailboxes, needs to be
   expanded to further minimize repetition

 * add more specialized template functions to avoid repetition

 * introduce ChangesTemplate[T] for *Changes structs
 * introduce jmap.Context to hold multiple parameters and shorten
   function calls

 * introduce SearchResultsTemplate
 * make the JMAP internal API a bit more future-proof by passing
   ObjectType objects instead of the JMAP namespaces

 * remove the new attempt to contain operations even further using the
   Factory objects

 * move CalendarEvent operations to its own file, like everything else

 * fix email tests

 * ignore WS error when closing an already closed connection
 * jmap: add UpdateContactCard and UpdateCalendarEvent funcs
 * use JSON marshalling and unmarshalling into a map for toPatch()
   implementations
 * add updating ContactCards and CalendarEvents to tests
 * add deleting ContactCards and CalendarEvents to tests
 * make query response totals work when the value is 0
 * tests: use CreateContactCard and CreateCalendarEvent funcs to create
   objects in tests instead of using a different JMAP stack that works
   with untyped maps
 * adds creating addressbooks, calendars, mailboxes

 * adds deleting mailbox, event, identity

 * adds modifying an email

 * introduce template functions for the Groupware API in templates.go,
   and use those in route function implementations whenever possible

 * add capability checking for mail, quota, blobs

 * adds Changes interface

 * adds JmapResponse interface
 * JMAP query limit of 0 is synonymous with "no limit", but we actually
   want to be able to perform queries without any results, for cases
   where we only want to count the total number of objects, and also
   because it makes more sense semantically

 * introduce query parameter validation checks, in order to only allow
   query parameters that are actually supported, which is going to be
   useful during development of clients
 * add query parameters 'anchor' and 'offset':
   - anchor is an object identifier
   - offset is a numeric offset relative to the anchor
 * the "position" attribute is always set to 0 when using an anchor for
   pagination, but it does not accurately reflect the position and thus
   we decided to remove the attribute from the resulting object for
   clarity

 * omit "canCalculateChanges" attribute when its value is "true", which
   is going to be the case in 100% of calls against Stalwart
 * in the JMAP API as well as in several places in the Groupware
   framework, use a single jmap.Response[T] object to return the
   payload, the language, the session state and the etag/state instead
   of individual multi-valued return values
@sonarqubecloud
Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
9 Security Hotspots
4.2% Duplication on New Code (required ≤ 3%)
C Reliability Rating on New Code (required ≥ A)
E Security Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

@codacy-production
Copy link
Copy Markdown

Not up to standards ⛔

🔴 Issues 18 critical · 25 high · 22 medium · 35 minor

Alerts:
⚠ 100 issues (≤ 0 issues of at least minor severity)

Results:
100 new issues

Category Results
BestPractice 1 high
ErrorProne 1 critical
1 high
Security 1 minor
23 high
17 critical
22 medium
CodeStyle 34 minor

View in Codacy

🟢 Metrics 3108 complexity · 4 duplication

Metric Results
Complexity 3108
Duplication 4

View in Codacy

AI Reviewer: first review requested successfully. AI can make mistakes. Always validate suggestions.

Run reviewer

TIP This summary will be updated as you push new changes.

Copy link
Copy Markdown

@codacy-production codacy-production Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR introduces the Groupware service architecture as a Proof of Concept. However, Codacy analysis indicates the project is not up to standards with 131 new issues identified. Several high-risk files lack test coverage, most notably the email handling logic in 'services/groupware/pkg/groupware/api_emails.go' (complexity: 302) and 'pkg/jmap/api_email.go' (complexity: 163).

There are several critical security flaws that should prevent merging in their current state: a potential XSS vulnerability in the auth-api, unsafe reflection in JMAP model examples, and the use of a Go version (1.25.0) with known vulnerabilities (CVE-2025-68121, CVE-2026-33814). Furthermore, the implementation of JMAP/JSContact data models and the Groupware service itself lack the unit and integration tests required for reliable maintenance.

About this PR

  • The implementation is currently labeled as a 'proof of concept' and is missing substantial unit, integration, and k6 black-box tests. This introduces significant technical debt into the main branch that must be addressed to ensure long-term maintainability.
  • API and system documentation are currently in a skeletal state, and no JIRA ticket was provided to track requirements. Documentation should be prioritized to support future development and integration.
6 comments outside of the diff
go.mod

line 3 🔴 HIGH RISK
Suggestion: Upgrade the Go version in the module configuration to address multiple security vulnerabilities in the standard library (including TLS and HTTP/2 issues). Note: Go 1.25.10 is recommended.

go 1.25.10
pkg/jmap/model_examples.go

line 38 🔴 HIGH RISK
Validate the 'name' parameter against a whitelist of explicitly permitted method names before calling 'MethodByName'. This ensures that only the intended API surface is reachable via reflection. This file is flagged as high-complexity (128) with no test coverage, making this reflection-based logic particularly risky.

Try running the following prompt in your IDE agent:

Identify the source of the 'name' variable used on line 38 of 'pkg/jmap/model_examples.go'. Create a whitelist of allowed method names and add a check to verify 'name' is in that list before proceeding with reflection.

services/auth-api/pkg/auth-api/authapi.go

line 328 🔴 HIGH RISK
Ensure the 'response' data is properly escaped. If this endpoint returns JSON, you should explicitly set the Content-Type header to 'application/json' before calling Write. If it is intended to be HTML, use the 'html/template' package for safe rendering.

Try running the following prompt in your IDE agent:

Examine 'authapi.go:328' and determine if 'response' contains user-controlled input. If so, refactor to set 'w.Header().Set("Content-Type", "application/json")' for JSON responses, or use 'html/template' for HTML output to prevent XSS.

services/groupware/pkg/groupware/framework.go

line 232 🔴 HIGH RISK
Suggestion: Explicitly set 'MinVersion' in the TLS configuration to at least 'tls.VersionTLS12' or 'tls.VersionTLS13' to ensure secure communication and prevent protocol downgrade attacks.

					tlsConfig := &tls.Config{InsecureSkipVerify: true, MinVersion: tls.VersionTLS12} // #nosec G402 insecure TLS is a configuration option for development
tests/groupware/groupware.ts

line 222 ⚪ LOW RISK
Nitpick: Replace loose equality checks with strict equality operators to prevent bugs caused by implicit type conversion.

      'vacation response has a notFound that only contains "singleton"': r => r.notFound && r.notFound.length === 1 && r.notFound[0] === 'singleton',

line 190-195 ⚪ LOW RISK
Suggestion: Extract this JMAP response validation logic into a shared helper function within the test suite to improve maintainability.

Try running the following prompt in your IDE agent:

In 'tests/groupware/groupware.ts', extract the duplicated object validation logic found at lines 190 and 213 into a reusable function that takes the response object as a parameter.

Test suggestions

  • Unit tests for JMAP, JSContact, and JSCalendar data model serialization/deserialization.
  • Integration tests validating the interaction between the Groupware service and the Stalwart mail server.
  • Verification that the services/groupware backend does not start by default.
  • Unit tests for new helper functions in pkg/structs.
  • Unit/Integration tests for high-complexity email handling in services/groupware/pkg/groupware/api_emails.go (Complexity: 302).
  • Unit/Integration tests for JMAP email API logic in pkg/jmap/api_email.go (Complexity: 163).
  • Unit/Integration tests for JMAP HTTP transport logic in pkg/jmap/http.go (Complexity: 109).
Prompt proposal for missing tests
Consider implementing these tests if applicable:
1. Unit tests for JMAP, JSContact, and JSCalendar data model serialization/deserialization.
2. Integration tests validating the interaction between the Groupware service and the Stalwart mail server.
3. Verification that the services/groupware backend does not start by default.
4. Unit tests for new helper functions in pkg/structs.
5. Unit/Integration tests for high-complexity email handling in services/groupware/pkg/groupware/api_emails.go (Complexity: 302).
6. Unit/Integration tests for JMAP email API logic in pkg/jmap/api_email.go (Complexity: 163).
7. Unit/Integration tests for JMAP HTTP transport logic in pkg/jmap/http.go (Complexity: 109).

TIP Improve review quality by adding custom instructions
TIP How was this review? Give us feedback

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Status:Needs-Review Type:Maintenance E.g. technical debt, packaging, etc.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants