Skip to content
Draft
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
f042c09
Initial plan
Copilot Dec 22, 2025
4273540
Add desktop/mobile app OAuth support with protocol handler redirect
Copilot Dec 22, 2025
cec5926
Add tests and documentation for desktop/mobile app OAuth support
Copilot Dec 22, 2025
f5dea14
Complete desktop/mobile app OAuth support implementation
Copilot Dec 22, 2025
b1d1350
Add native desktop/mobile client detection and redirect handling
insoln Dec 22, 2025
b5c9b33
Fix desktop app OAuth - use window.open() to trigger external browser
Copilot Dec 23, 2025
7ce164e
Remove unused variable in window.open() call
Copilot Dec 23, 2025
9803693
Add debug logging for isMobile parameter detection
Copilot Dec 23, 2025
fa838c5
Add troubleshooting section and verify code implementation
Copilot Dec 23, 2025
319f1c1
Improve mobile/desktop client handling in OAuth flow and enhance exte…
insoln Dec 23, 2025
808e7d0
Implement desktop login finalization and enhance mobile/desktop OAuth…
insoln Dec 23, 2025
35fdcfb
feat: enhance OIDC login diagnostics and UI
insoln Dec 23, 2025
25e19f3
feat: enhance OIDC redirect handling and add mobile complete route test
insoln Dec 23, 2025
d0071eb
fix: standardize formatting in TestHandleMobileCompleteTrailingSlash
insoln Dec 23, 2025
4a75550
fix: address critical security issues from PR review
Copilot Dec 26, 2025
40a04c2
test: add comprehensive unit tests for redirect and sanitization func…
Copilot Dec 26, 2025
bf7a33e
feat: add accessibility improvements to LoginPanel diagnostics
Copilot Dec 26, 2025
7c3776c
fix: add missing CSS feedback class and improve test realism
Copilot Dec 26, 2025
8f2ca6b
fix: trigger deep link automatically on complete page load
Copilot Dec 26, 2025
3089443
test: add E2E tests for desktop client OAuth flow
Copilot Dec 26, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,18 @@ Securely connect Mattermost Server v9.x+ to modern OpenID Connect providers (Key
## What you get

- ✅ Full OIDC login flow with automatic user provisioning and optional admin promotion via client roles.
- ✅ Desktop/Mobile app support with browser-based authentication and protocol handler redirect.
- ✅ Ready-made Docker stack (Mattermost + Keycloak + Postgres + proxy) for demos, QA, and CI.
- ✅ Proxy recipe that rewrites `/login` to the plugin route without breaking API clients.
- ✅ Playwright regression suite that mirrors the documented installation steps.

## Choose your deployment path

Use the [User Guide](docs/USER_GUIDE.md) for step-by-step instructions covering two supported scenarios:
Use the [User Guide](docs/USER_GUIDE.md) for step-by-step instructions covering three supported scenarios:

1. **Existing Mattermost instances** – upload `mm-oidc.tar.gz`, configure the IdP client, and validate the flow from the System Console.
2. **Proxy-assisted login** – drop the maintained Nginx container (or Helm/K8s manifests) in front of Mattermost so `/login` automatically redirects into the plugin without touching server code.
3. **Desktop/Mobile applications** – enable browser-based OAuth authentication for Mattermost desktop and mobile apps with automatic protocol handler redirect.

Need the standalone Docker stack or automation workflows? Jump to [docs/DEVELOPER_GUIDE.md](docs/DEVELOPER_GUIDE.md).

Expand Down
31 changes: 30 additions & 1 deletion deploy/mattermost-proxy/nginx.conf
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,30 @@ http {
"~*application/json" 0;
}

# Map User-Agent strings to detect native desktop/mobile clients that need a custom login flow
map $http_user_agent $native_login_suffix {
default "";
~*Mattermost "&isMobile=true";
}

# Track the last meaningful in-app path so we can restore it after SSO
map $request_uri $oidc_trackable_request {
~^/$ "";
~^/login "";
default $request_uri;
}

map "$request_method|$oidc_trackable_request" $oidc_redirect_hint {
default "";
~^(GET|HEAD)\|.+$ $oidc_trackable_request;
}

map $oidc_redirect_hint $oidc_redirect_cookie {
default "";
"" "";
~.+ "MMOIDC_REDIRECT=$oidc_redirect_hint; Path=/; Max-Age=900; HttpOnly; SameSite=Lax";
}

# Upstream for Mattermost
upstream mattermost {
server mattermost:8065;
Expand Down Expand Up @@ -113,16 +137,21 @@ http {

# All other locations - check auth and redirect if needed
location / {
add_header Set-Cookie $oidc_redirect_cookie always;

# Build a composite condition using map variables
# Redirect when all conditions are true:
# - auth_redirect = 1 (no cookie)
# - method_allows_redirect = 1 (GET request)
# - accept_allows_redirect = 1 (not an API client)
set $should_redirect "${auth_redirect}${method_allows_redirect}${accept_allows_redirect}";

# Build the login redirect target, appending the mobile flag for native clients when necessary

Copilot AI Dec 26, 2025

Copy link

Choose a reason for hiding this comment

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

The $native_login_suffix is concatenated directly into the redirect URL without URL encoding. If a User-Agent contains special characters that match the regex but also include characters like &, ?, or #, it could break the URL structure.

While the current implementation only appends the literal string "&isMobile=true", consider using nginx's $arg_* or ensuring any future modifications to this logic properly encode values. The current implementation is safe but fragile to changes.

For improved robustness, document that this value must be a literal string and should not be dynamically constructed from user input.

Suggested change
# Build the login redirect target, appending the mobile flag for native clients when necessary
# Build the login redirect target, appending an optional suffix for native clients.
# SECURITY NOTE:
# - $native_login_suffix MUST be a literal, URL-safe string (e.g., "&isMobile=true" or an empty string).
# - It MUST NOT be dynamically constructed from user input (headers, query params, etc.),
# because it is concatenated directly into the redirect URL without additional encoding.

Copilot uses AI. Check for mistakes.
set $oidc_login_redirect "$scheme://$http_host/plugins/com.mm.oidc/login?redirect_to=$request_uri$native_login_suffix";

# Perform the redirect to OIDC login when all conditions match (111)
if ($should_redirect = "111") {
return 302 $scheme://$http_host/plugins/com.mm.oidc/login?redirect_to=$request_uri;
return 302 $oidc_login_redirect;
}

# Otherwise, proxy to Mattermost
Expand Down
4 changes: 4 additions & 0 deletions docs/PROXY_GUIDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,10 @@ Add explicit `location` blocks or conditional checks for:

These carve-outs keep API clients, automation, health probes, and WebSockets functional without interference.

### Native desktop/mobile detection

The bundled config in [deploy/mattermost-proxy/nginx.conf](../deploy/mattermost-proxy/nginx.conf) now inspects the `User-Agent` header for the `Mattermost` token. When detected, the proxy automatically appends `isMobile=true` to the `/plugins/com.mm.oidc/login` redirect so the plugin switches to the desktop/mobile handshake (launching the system browser and handing tokens back via `mattermost://`). Servers that maintain their own ingress layer should replicate the same behavior or adjust the regex list if your fleet uses custom User-Agent strings.

### Optional Traefik pattern

Traefik can reproduce the same behavior using a custom middleware plugin (for cookie inspection) or a combination of RedirectRegex and AllowList middleware. See the snippets in the original research section below or adapt [Traefik documentation](https://doc.traefik.io/traefik/) to your needs.
Expand Down
93 changes: 93 additions & 0 deletions docs/USER_GUIDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -139,3 +139,96 @@ spec:
- If you run multiple Mattermost instances behind the proxy, configure sticky sessions or an external session store so `MMAUTHTOKEN` cookies stay valid across hosts.

---

## 3. Using the plugin with Mattermost Desktop App

The plugin supports authentication via the Mattermost Desktop application on Windows, macOS, and Linux. The desktop app opens the OAuth flow in your system's default browser, then redirects back to the app after successful authentication.

### Prerequisites

- Mattermost Desktop app v5.0+ installed and configured
- Plugin installed and configured as described in section 1
- System browser (Chrome, Firefox, Safari, Edge) available

### How it works

1. **Desktop app initiates login**: When you click "Sign in with SSO" or similar in the desktop app, it navigates to the plugin's login URL.
2. **Detection and external browser**: The plugin detects desktop/mobile clients (via the `isMobile=true` query parameter, which can be added by the client or by a proxy inspecting the User-Agent). For desktop clients, instead of a simple HTTP redirect, the plugin serves an HTML page that opens the OAuth provider (Keycloak) in your system's external browser using JavaScript `window.open()`.
3. **Browser authentication**: You complete the OIDC authentication flow in your external browser (which allows using saved passwords, password managers, security keys, etc.).
4. **Protocol handler redirect**: After successful authentication, the browser redirects to a `mattermost://` URL that the desktop app is registered to handle.
5. **Session handoff**: The desktop app receives the authentication tokens and establishes your session.

### Configuration

The plugin automatically detects desktop/mobile clients when the `isMobile=true` query parameter is present in the login URL.

**Option 1: Using the bundled proxy (Recommended)**
If you front Mattermost with the bundled nginx proxy (or copy its rules), desktop/mobile requests are automatically detected via `User-Agent` header, and the proxy appends `isMobile=true` to `/plugins/com.mm.oidc/login` before forwarding to the plugin. This ensures OAuth opens in external browser without client-side changes.

**Option 2: Without proxy**
The desktop app needs to add `?isMobile=true` to the login URL when initiating OAuth flows. Contact your desktop app administrator or configure your deployment to append this parameter for Mattermost desktop clients.

### Usage

1. **Launch the Mattermost Desktop app**
2. **Add or select your server**: Enter your Mattermost server URL (e.g., `https://chat.example.com`)
3. **Click to authenticate**: The app will detect that SSO is available and open your system browser
4. **Complete authentication**: Log in through your Identity Provider in the browser
5. **Return to app**: After successful authentication, you'll see a "Redirecting to Mattermost" page that automatically opens the desktop app
- If automatic redirect doesn't work, click the "Click here to open Mattermost" button

### Troubleshooting

**Problem**: Browser redirects to Mattermost but desktop app doesn't open

**Solutions**:
- Ensure the desktop app is installed and running
- Check that the `mattermost://` protocol handler is registered (this happens automatically during installation)
- On Windows: Check Windows Settings → Apps → Default Apps → Choose default apps by protocol
- On macOS: The protocol handler should be registered automatically; try reinstalling the desktop app if it's not working
- On Linux: Check your desktop environment's protocol handler configuration

**Problem**: Desktop app opens but login doesn't complete

**Solutions**:
- Ensure you're running Desktop app v5.0 or newer
- Check that the server URL in the desktop app matches your plugin's `Redirect URL` configuration
- Review Mattermost server logs for any error messages
- Try clearing the desktop app's cache (File → Settings → Clear Cache and Restart)

**Problem**: "Missing authentication parameters" error

**Solutions**:
- This may indicate that the desktop app isn't properly passing the session tokens
- Try logging in via the web browser first to verify the plugin is working correctly
- Check that your Mattermost server's `SiteURL` is configured correctly in System Console

**Problem**: Plugin changes not taking effect / still seeing HTTP 302 redirects

**Solutions**:
- **Rebuild the plugin** after code changes:
```bash
cd /path/to/mm-oidc
make server-build
make package # creates plugin bundle
```
- **Redeploy to Mattermost**:
- Upload new plugin bundle via System Console → Plugin Management
- Or restart development stack: `make dev-down && make dev-up`
- **Verify deployment**:
- Check plugin version in System Console matches your build
- Review Mattermost logs for `"handleLogin called"` messages
- Clear browser cache and test again
- **Expected behavior**: When `isMobile=true` is present, should see HTTP 200 with HTML page (not HTTP 302 redirect)

### Security notes

- **Browser-based authentication is more secure**: The desktop app opens OAuth in your system browser (not an embedded webview), which allows you to:
- Verify you're on the correct login page
- Use browser password managers
- Use hardware security keys (FIDO2/WebAuthn)
- Reuse existing authenticated sessions
- **Protocol handler security**: The `mattermost://` protocol ensures that authentication tokens are only passed to the legitimate Mattermost desktop application installed on your system
- **Short-lived tokens**: Authentication tokens passed via the protocol handler are short-lived and single-use

---
14 changes: 14 additions & 0 deletions e2e/tests/proxy-redirect.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -204,4 +204,18 @@ test.describe('NGINX Proxy Redirect Behavior', () => {
expect(location).not.toMatch(/\/plugins\/com\.mm\.oidc\/login/);
}
});

test('should append isMobile flag for native desktop clients', async ({ request }) => {
const response = await request.get(PROXY_BASE_URL + '/', {
headers: {
'User-Agent': 'MattermostDesktop/6.0.2'
},
maxRedirects: 0,
failOnStatusCode: false,
});

expect(response.status()).toBe(302);
const location = response.headers()['location'] ?? '';
expect(location).toMatch(/isMobile=true/);
});
});
7 changes: 4 additions & 3 deletions scripts/dev-bootstrap.sh
Original file line number Diff line number Diff line change
Expand Up @@ -127,9 +127,10 @@ ensure_plugin_installed() {
return 0
fi

if ! run_mmctl plugin list | grep -q "${PLUGIN_ID}"; then
log "Uploading plugin ${PLUGIN_ID} from ${PLUGIN_CONTAINER_PATH} (force replace)."
run_mmctl plugin add --force "${PLUGIN_CONTAINER_PATH}" >/dev/null
log "Uploading plugin ${PLUGIN_ID} from ${PLUGIN_CONTAINER_PATH} (force replace)."
if ! run_mmctl plugin add --force "${PLUGIN_CONTAINER_PATH}" >/dev/null; then
log "Failed to upload plugin ${PLUGIN_ID}." >&2
exit 1
fi
Comment on lines +134 to 138

Copilot AI Dec 26, 2025

Copy link

Choose a reason for hiding this comment

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

The change removes the conditional check if ! run_mmctl plugin list | grep -q "${PLUGIN_ID}" that would skip uploading if the plugin already exists. Now it unconditionally uploads and replaces the plugin every time.

While this ensures the latest version is always deployed (which is good for development), it also means the plugin will be force-replaced even when it's not necessary, potentially causing brief service interruptions.

This is probably intentional for the development workflow (ensuring fresh deploys after code changes), but consider adding a comment explaining this behavior, especially since the PR description mentions that users need to rebuild/redeploy for changes to take effect.

Copilot uses AI. Check for mistakes.

log "Enabling plugin ${PLUGIN_ID}."
Expand Down
4 changes: 4 additions & 0 deletions scripts/test-proxy.sh
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,10 @@ run_test "Root URL redirect points to OIDC login" \
"curl -s -D - -o /dev/null '${BASE_URL}/' | grep -i location" \
'/plugins/com.mm.oidc/login'

run_test "Desktop/mobile client receives mobile flag" \
"curl -s -D - -o /dev/null -H 'User-Agent: MattermostDesktop/6.0.2' '${BASE_URL}/' | grep -i location" \
'isMobile=true'

echo ""
echo "==> Scenario 2: API endpoints should NOT redirect"
run_test "API endpoint returns 401 or 200, not redirect" \
Expand Down
6 changes: 4 additions & 2 deletions server/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,14 @@ go 1.22.0

toolchain go1.24.2

require github.com/mattermost/mattermost/server/public v0.1.10
require (
github.com/coreos/go-oidc/v3 v3.9.0
github.com/mattermost/mattermost/server/public v0.1.10
)

require (
Comment on lines +8 to 12

Copilot AI Dec 26, 2025

Copy link

Choose a reason for hiding this comment

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

The dependency github.com/coreos/go-oidc/v3 has been moved from indirect dependencies to direct dependencies. However, I cannot see any new direct imports of this package in the diff.

This change suggests the package was already being used (likely in existing code) and is now being properly declared as a direct dependency, which is good for dependency management. But if this is truly a new dependency for this PR, please verify that it's actually being imported and used in the codebase.

Suggested change
github.com/coreos/go-oidc/v3 v3.9.0
github.com/mattermost/mattermost/server/public v0.1.10
)
require (
github.com/mattermost/mattermost/server/public v0.1.10
)
require (
github.com/coreos/go-oidc/v3 v3.9.0 // indirect

Copilot uses AI. Check for mistakes.
filippo.io/edwards25519 v1.1.0 // indirect
github.com/blang/semver/v4 v4.0.0 // indirect
github.com/coreos/go-oidc/v3 v3.9.0 // indirect
github.com/dyatlov/go-opengraph/opengraph v0.0.0-20220524092352-606d7b1e5f8a // indirect
github.com/fatih/color v1.18.0 // indirect
github.com/francoispqt/gojay v1.2.13 // indirect
Expand Down
2 changes: 2 additions & 0 deletions server/oidc.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ type authSession struct {
Nonce string `json:"nonce"`
CodeVerifier string `json:"code_verifier"`
CreatedAt int64 `json:"created_at"`
IsMobile bool `json:"is_mobile"`
RedirectTo string `json:"redirect_to,omitempty"`
}

func (s *authSession) isExpired(now time.Time) bool {
Expand Down
Loading
Loading