Skip to content

Commit a9013af

Browse files
carlhoerbergclaude
andcommitted
Encapsulate User permissions with immutable update methods
Add protected methods for permission management in User class and update all access points to use the new API for better data integrity. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 59f1825 commit a9013af

File tree

9 files changed

+61
-29
lines changed

9 files changed

+61
-29
lines changed

src/lavinmq/amqp/connection_factory.cr

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ module LavinMQ
166166
open = AMQP::Frame.from_io(socket) { |f| f.as(AMQP::Frame::Connection::Open) }
167167
vhost_name = open.vhost.empty? ? "/" : open.vhost
168168
if vhost = @vhosts[vhost_name]?
169-
if user.permissions[vhost_name]?
169+
if user.permission?(vhost_name)
170170
if vhost.max_connections.try { |max| vhost.connections.size >= max }
171171
log.warn { "Max connections (#{vhost.max_connections}) reached for vhost #{vhost_name}" }
172172
reply_text = "access to vhost '#{vhost_name}' refused: connection limit (#{vhost.max_connections}) is reached"

src/lavinmq/auth/user.cr

Lines changed: 38 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ require "../tag"
66
module LavinMQ
77
class User
88
include SortableJSON
9-
getter name, password, permissions
9+
getter name, password
1010
property tags, plain_text_password
1111
alias Permissions = NamedTuple(config: Regex, read: Regex, write: Regex)
1212

@@ -130,24 +130,58 @@ module LavinMQ
130130
end
131131

132132
def can_write?(vhost, name) : Bool
133-
perm = permissions[vhost]?
133+
perm = @permissions[vhost]?
134134
perm ? perm_match?(perm[:write], name) : false
135135
end
136136

137137
def can_read?(vhost, name) : Bool
138-
perm = permissions[vhost]?
138+
perm = @permissions[vhost]?
139139
perm ? perm_match?(perm[:read], name) : false
140140
end
141141

142142
def can_config?(vhost, name) : Bool
143-
perm = permissions[vhost]?
143+
perm = @permissions[vhost]?
144144
perm ? perm_match?(perm[:config], name) : false
145145
end
146146

147147
def can_impersonate?
148148
@tags.includes? Tag::Impersonator
149149
end
150150

151+
protected def set_permission(vhost : String, config : Regex, read : Regex, write : Regex)
152+
set_permission(vhost, {config: config, read: read, write: write})
153+
end
154+
155+
protected def set_permission(vhost : String, perms : Permissions)
156+
new_permissions = @permissions.dup
157+
new_permissions[vhost] = perms
158+
@permissions = new_permissions
159+
perms
160+
end
161+
162+
protected def remove_permission(vhost : String)
163+
new_permissions = @permissions.dup
164+
perm = new_permissions.delete(vhost)
165+
@permissions = new_permissions
166+
perm
167+
end
168+
169+
def has_permission?(vhost : String) : Bool
170+
@permissions.has_key? vhost
171+
end
172+
173+
def permission?(vhost : String) : Permissions?
174+
@permissions[vhost]?
175+
end
176+
177+
def permitted_vhosts : Array(String)
178+
@permissions.keys
179+
end
180+
181+
def permissions : Enumerable(Tuple(String, Permissions))
182+
@permissions.each
183+
end
184+
151185
private def parse_permissions(pull)
152186
pull.read_object do |vhost|
153187
config = read = write = /^$/

src/lavinmq/auth/user_store.cr

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -76,18 +76,18 @@ module LavinMQ
7676
def add_permission(user, vhost, config, read, write)
7777
perm = {config: config, read: read, write: write}
7878
@lock.synchronize do
79-
if @users[user].permissions[vhost]? == perm
79+
if @users[user].permission?(vhost) == perm
8080
return perm
8181
end
82-
@users[user].permissions[vhost] = perm
82+
@users[user].set_permission(vhost, perm)
8383
save!
8484
perm
8585
end
8686
end
8787

8888
def rm_permission(user, vhost)
8989
@lock.synchronize do
90-
if perm = @users[user].permissions.delete vhost
90+
if perm = @users[user].remove_permission vhost
9191
Log.info { "Removed permissions for user=#{user} on vhost=#{vhost}" }
9292
save!
9393
perm
@@ -98,7 +98,7 @@ module LavinMQ
9898
def rm_vhost_permissions_for_all(vhost)
9999
@lock.synchronize do
100100
@users.each_value do |user|
101-
user.permissions.delete(vhost)
101+
user.remove_permission(vhost)
102102
end
103103
save!
104104
end
@@ -175,7 +175,7 @@ module LavinMQ
175175
private def create_direct_user
176176
@users[DIRECT_USER] = User.create_hidden_user(DIRECT_USER)
177177
perm = {config: /.*/, read: /.*/, write: /.*/}
178-
@users[DIRECT_USER].permissions["/"] = perm
178+
@users[DIRECT_USER].set_permission("/", perm)
179179
end
180180

181181
def save!

src/lavinmq/http/controller.cr

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -221,14 +221,14 @@ module LavinMQ
221221
def vhosts(user : User)
222222
@amqp_server.vhosts.each_value.select do |v|
223223
full_view_vhosts_access = user.tags.any? { |t| t.administrator? || t.monitoring? }
224-
amqp_access = user.permissions.has_key?(v.name)
224+
amqp_access = user.has_permission?(v.name)
225225
full_view_vhosts_access || (amqp_access && !user.tags.empty?)
226226
end
227227
end
228228

229229
private def refuse_unless_vhost_access(context, user, vhost)
230230
return if user.tags.any? &.administrator?
231-
unless user.permissions.has_key?(vhost)
231+
unless user.has_permission?(vhost)
232232
Log.warn { "user=#{user.name} does not have permissions to access vhost=#{vhost}" }
233233
access_refused(context)
234234
end

src/lavinmq/http/controller/connections.cr

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ module LavinMQ
88
if user.tags.any? { |t| t.administrator? || t.monitoring? }
99
@amqp_server.connections
1010
else
11-
vhosts = user.permissions.keys
11+
vhosts = user.permitted_vhosts
1212
@amqp_server.connections.select &.vhost.name.in?(vhosts)
1313
end
1414
end

src/lavinmq/http/controller/definitions.cr

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -192,11 +192,10 @@ module LavinMQ
192192
configure = p["configure"].as_s
193193
read = p["read"].as_s
194194
write = p["write"].as_s
195-
@amqp_server.users[user].permissions[vhost] = {
196-
config: Regex.new(configure),
197-
read: Regex.new(read),
198-
write: Regex.new(write),
199-
}
195+
@amqp_server.users[user].set_permission(vhost,
196+
Regex.new(configure),
197+
Regex.new(read),
198+
Regex.new(write))
200199
end
201200
@amqp_server.users.save!
202201
end

src/lavinmq/http/controller/permissions.cr

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ module LavinMQ
3939
refuse_unless_administrator(context, user(context))
4040
with_vhost(context, params) do |vhost|
4141
u = user(context, params, "user")
42-
perm = u.permissions[vhost]?
42+
perm = u.permission?(vhost)
4343
not_found(context) unless perm
4444
u.permissions_details(vhost, perm).to_json(context.response)
4545
end
@@ -56,7 +56,8 @@ module LavinMQ
5656
unless config && read && write
5757
bad_request(context, "Fields 'configure', 'read' and 'write' are required")
5858
end
59-
is_update = @amqp_server.users[u.name].permissions[vhost]?
59+
user = @amqp_server.users[u.name]
60+
is_update = user.permission?(vhost)
6061
@amqp_server.users
6162
.add_permission(u.name, vhost, Regex.new(config), Regex.new(read), Regex.new(write))
6263
context.response.status_code = is_update ? 204 : 201

src/lavinmq/http/controller/vhosts.cr

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ module LavinMQ
7474
with_vhost(context, params, "name") do |vhost|
7575
@amqp_server.users.compact_map do |_, u|
7676
next if u.hidden?
77-
u.permissions[vhost]?.try { |p| u.permissions_details(vhost, p) }
77+
u.permission?(vhost).try { |p| u.permissions_details(vhost, p) }
7878
end.to_json(context.response)
7979
end
8080
end

src/lavinmq/mqtt/connection_factory.cr

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -62,14 +62,12 @@ module LavinMQ
6262
username = username[split_pos + 1..]
6363
end
6464

65-
user = @authenticator.authenticate(username, password)
66-
return unless user
67-
has_vhost_permissions = user.try &.permissions.has_key?(vhost)
68-
return unless has_vhost_permissions
69-
broker = @brokers[vhost]?
70-
return unless broker
71-
72-
{user, broker}
65+
if broker = @brokers[vhost]?
66+
if user = @authenticator.authenticate(username, password)
67+
user.permission?(vhost) || return
68+
{user, broker}
69+
end
70+
end
7371
end
7472

7573
def assign_client_id(packet)

0 commit comments

Comments
 (0)