Shopify / Shopify/shipit-engine

Problems with using standandalone pubsubstub with rails 6 redis session store

Open
#1,082 2 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Ruby
Stars
1.5k
Forks
154
Avg merge
2h 26m
Merged PRs (30d)
3

Description

We recently upgraded an instance of shipit-engine to 0.31 (and now running shipit-engine master)

When running pubsubstub in a separate app, we don't want to leave the /events endpoint exposed, so our goal is to return 403s and short circuit the pubsubstub server with a middleware if we get a request that doesn't map to the session that was created from the rails app.

Unfortunately, we are unable to use the same session cookie created from the rails server in the pubsubstub server

we ran into two problems so far:

  1. Doing request.session[:init] = true in the pubsubstub app was blowing up with an error. I did some digging around and found that this was an issue related to an incompatibility between redis-rb 4.2 and redis-store. This was fixed by @casperisfine very recently here: https://github.com/redis-store/redis-store/pull/334. We are running this patch gem 'redis-store', github: 'redis-store/redis-store' to get around this. I think this is needed because the session is uninitialized in rack without it because of some lazy loading.

  2. It seems like rack/session/redis is creating a new session with nothing in it as opposed to reading rails session. session[:user_id] is not defined when I try to print it from the pubsubstub application after the Rack::Session::Redismiddleware executed.

@casperisfine i hate to bug you again but you know probably redis-store, pubsubstub and shipit better than anyone - can you please share how you would implement UserRequiredMiddleware today if we are storing sessions from the rails side using the redis_cache_store as the `session_store?

We run pubsubstub in its own process/container like this:

bundle exec puma --config pubsubstub/puma_config.rb pubsubstub/config.ru --port 3000 environment $RAILS_ENV

pubsubstub/puma_config.rb:

#!/usr/bin/env puma

# Tells puma to not wait on clients to sutdown
force_shutdown_after 0

# Number of processes
workers 2

port ENV.fetch("PORT") { 3000 }

# min, max threads per workers
threads 32, 256

This is how we are setting up the session store for the rails application:

# config/initializers/session_store.rb
Shipit::Application.config.session_store :cache_store,
    servers: ["#{ENV['REDIS_URL']}/0/session"],
    key: '_shipit_session',
    expires_in: 60.minutes,
    threadsafe: false,
    signed: true,
    secure: true,
    http_only: true

How the cache store is set up:

# config/environments.production.rb
config.cache_store = :redis_cache_store, { url: Shipit.redis_url.to_s, expires_in: 90.minutes }

pubsubstub/config.ru

require 'rack'
require 'pubsubstub'
require 'rack/session/redis'

class HealthCheck
  def initialize(app)
    @app = app
  end

  def call(env)
    if env['PATH_INFO'] == '/events/healthcheck'
      return [200, {'Content-Type' => 'text/plain'}, %w(OK)]
    end

    return @app.call(env) 
end

class UserRequiredMiddleware
  def initialize(app)
    @app = app
  end

  def call(env)
    request = Rack::Request.new(env)
    # this was throwing before the redis-store patch
    request.session[:init] = true 

    # this condition is never true 
    if request.session[:user_id]
      return @app.call(env) 
    end

    [403, {"Content-Type" => "text/html"}, ["Log into Shipit first"]]
  rescue
    [500, {"Content-Type" => "text/html"}, ["Server Error"]]
  end
end

Pubsubstub.redis_url = ENV['REDIS_URL']

use HealthCheck
use Rack::Session::Redis, {redis_server: "#{ENV['REDIS_URL']}/0/session", key: "_shipit_session"}
use UserRequiredMiddleware
run Pubsubstub::StreamAction.new

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by reading config/initializers/session_store.rb, config/environments.production.rb, and pubsubstub/config.ru, then trace how the Rails cache-backed session is created and how Rack::Session::Redis reads it. Reproduce the missing user_id behavior with the shown Redis URL, cookie key, and middleware order. Done means the standalone app can reliably identify the Rails session, or the incompatibility and required configuration are documented.

Written by the indexing model from the issue text.

Assessment

Tech stack
rails, redis, ruby
Domain
authentication, backend
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.