discourse icon indicating copy to clipboard operation
discourse copied to clipboard

FEATURE: adding profile param to s3_options function

Open ducks opened this issue 4 months ago • 1 comments

Overview

This PR implements AWS IAM role assumption for S3 operations in Discourse, replacing long-lived static access keys with temporary credentials obtained through role assumption. This significantly reduces security risk by eliminating the need for powerful, permanent credentials while maintaining full S3 functionality.

The Problem

The current implementation relies on static access keys with broad S3 permissions that never expire. This creates several security concerns:

  • Static keys with extensive permissions are permanent attack vectors if compromised
  • Key rotation requires manual intervention and service restarts
  • No ability to scope permissions to specific operations or time windows
  • Difficult to audit which operations were performed by which service

The existing s3_use_iam_profile setting attempted to address this but was insufficient because:

  • It only supported instance profile credentials, not role assumption
  • It couldn't handle credential_source = Environment for role assumption
  • The AWS SDK credential chain has a bug preventing proper role assumption with default profiles

The Solution

This PR enables proper IAM role assumption, allowing Discourse to:

  • Use minimal base credentials (or instance profiles) to assume roles with scoped permissions
  • Obtain temporary credentials that automatically expire
  • Implement least-privilege access per operation type
  • Leverage AWS STS for credential management and auditing

Additionally, the AWS SDK credential chain has a documented bug where credential_source = Environment doesn't work with default profiles, requiring explicit profile parameters for reliable role assumption.

Key Changes

Simplified Credential Handling

Replaces boolean flag with intelligent credential chain:

# Before: Binary choice via s3_use_iam_profile
unless obj.s3_use_iam_profile
  opts[:access_key_id] = obj.s3_access_key_id
  opts[:secret_access_key] = obj.s3_secret_access_key
end

# After: Flexible credential discovery
if obj.respond_to?(:s3_profile) && obj.s3_profile.present?
  opts[:profile] = obj.s3_profile
elsif obj.s3_access_key_id.present? && obj.s3_secret_access_key.present?
  opts[:access_key_id] = obj.s3_access_key_id
  opts[:secret_access_key] = obj.s3_secret_access_key
end
# If neither: AWS SDK auto-discovers (IAM roles, instance profiles, etc.)

New Configuration Option

  • DISCOURSE_S3_PROFILE (GlobalSetting only) - AWS profile name for role assumption
    • Example: DISCOURSE_S3_PROFILE=file-uploads
    • Works with AWS config files (~/.aws/config) for role assumption

Removed Configuration

  • s3_use_iam_profile - No longer needed, credential method is auto-detected
  • Simplified validation - only requires bucket name, not credential validation

Authentication Flow

The credential chain now works as:

  1. Profile-based (new): If s3_profile is set → use that AWS profile
  2. Key-based (existing): If both access keys provided → use explicit credentials
  3. Auto-discovery (improved): If neither → let AWS SDK discover (IAM roles, instance profiles, ECS task roles, etc.)

Benefits

  • Enables IAM role assumption with temporary credentials instead of static keys
  • Reduces blast radius of credential compromise through time-limited credentials
  • Maintains backward compatibility with key-based auth for migration period
  • Enables AWS SDK auto-discovery when appropriate
  • Simplifies configuration (removes s3_use_iam_profile setting)
  • Improves auditability through AWS STS

Testing

  • All existing tests passing (including previously failing secure upload specs)
  • Jenkins CI passing (first time in over a month)
  • Uploads working consistently in test cluster

Configuration Examples

Using IAM Role Assumption (New)

# app.yml
DISCOURSE_S3_PROFILE: "file-uploads"
# ~/.aws/config
[profile file-uploads]
role_arn = arn:aws:iam::123456789012:role/DiscourseUploads
credential_source = Environment

Using Static Keys (Existing)

# No changes needed - still works with s3_access_key_id and s3_secret_access_key

Using IAM Instance Profile (Improved)

# Just omit both profile and keys - SDK auto-discovers
# s3_access_key_id and s3_secret_access_key left blank

Migration Path

Existing deployments continue to work without changes:

  • Key-based auth: No action needed
  • IAM profile users: Can remove s3_use_iam_profile = true, behavior unchanged
  • Want role assumption: Set DISCOURSE_S3_PROFILE and configure AWS profile

Related Work

This implementation is based on extensive investigation of the AWS SDK Ruby credential chain behavior documented in a three-part blog series. The AWS maintainers declined to fix the credential chain bug, so explicit profile passing is the only reliable way to use credential_source = Environment with role assumption.

ducks avatar Aug 18 '25 16:08 ducks

s3_file_uploads_profile - Profile name for file uploads (defaults to "file-uploads")

The default is empty:

  s3_file_uploads_profile:
    default: ""
    secret: true

I don't think there should be a default.

I also don't think these need to be marked secret.

Supermathie avatar Sep 29 '25 17:09 Supermathie