Skip to content

API-only access, per issue #150#222

Merged
Pita merged 5 commits intoether:masterfrom
jhollinger:master
Nov 26, 2011
Merged

API-only access, per issue #150#222
Pita merged 5 commits intoether:masterfrom
jhollinger:master

Conversation

@jhollinger
Copy link
Copy Markdown
Contributor

This implements an api-only mode, per issue #150. Actually it splits the issue into two settings which may be used independently or in concert:

requireSession: requires users to have a session (disables non-group pads)
editOnly: users may only edit existing pads - new pads can only be created via the api

Comment thread node/server.js Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This part shouldn't be necessary, cause it should reject the user on the socket.io connection try. I think we shouldn't have 2 positions in the code where we check permissions. Everything else looks fine. Will have a close look later

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Missed that. I'll remove the redundant checks.

@jhollinger
Copy link
Copy Markdown
Contributor Author

The permissions checks I've added in SecurityManager.checkAccess appear to make an existing bug more prominent. When checking to see if a pad exists (because of the editOnly flag), a padID of "test 1" is passed to padManager.doesPadExists as "test%201". This results in the user being refused access to the timeline. This bug does not present when it's a group pad, however.

I think there are several open issues related to this (#135, #215 and #136). Just thought I'd point out that this feature will make the bug more prominent.

@Pita
Copy link
Copy Markdown
Contributor

Pita commented Nov 26, 2011

@jhollinger are you sure the space bug isn't fixed already? This should have fixed it Pita/etherpad-lite@5ea8dd6

Comment thread settings.json.template Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

editOnly is by default true? why?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Must have accidentally copied that from my settings.json file. I have corrected it.

@Pita
Copy link
Copy Markdown
Contributor

Pita commented Nov 26, 2011

Why did you decide to do 2 options? What would be the usecase where you would enable one but not the other?

@jhollinger
Copy link
Copy Markdown
Contributor Author

I thought it could be useful to require authentication (session). But once
you're signed in, you can edit or create any pads in your group(s). This
forces all pads to be in groups.

Limiting the pads which can be created (signed in or not) is quite another
thing, I think. This prevents people from creating arbitrary pads.

I'm open to combining them, if you think that's clearer.
On Nov 25, 2011 7:25 PM, "Peter 'Pita' Martischka" <
reply@reply.github.com>
wrote:

Why did you decide to do 2 options? What would be the usecase where you
would enable one but not the other?


Reply to this email directly or view it on GitHub:
https://github.com/Pita/etherpad-lite/pull/222#issuecomment-2879173

@jhollinger
Copy link
Copy Markdown
Contributor Author

@Pita Yes, the spacing issue seems fixed now. I had simply not pulled that change into my branch yet.

@kunoFM
Copy link
Copy Markdown

kunoFM commented Nov 26, 2011

Option "editOnly" makes it possible to control the creation of new pads. External application can use API for creation. But users can then use the pads both "embedded" or stand-alone and can invite other users, who might not be users of the external application, to contribute to the pad.

Option "requireSession" limits the whole access to pads to the users of an external application.

Both use cases seem useful and make sense to me, as they address different scenarios.

Pita added a commit that referenced this pull request Nov 26, 2011
@Pita Pita merged commit 3aacc0a into ether:master Nov 26, 2011
@Pita
Copy link
Copy Markdown
Contributor

Pita commented Nov 26, 2011

merged, thx

JohnMcLear added a commit to JohnMcLear/etherpad that referenced this pull request Apr 19, 2026
Pure primaries like #ff0000 cannot clear WCAG AA (4.5:1) against either
ether#222 or #fff — the best either can do is ~4.0:1. No text-colour choice
alone fixes that; bg clamping would be a separate concern. The test
should therefore verify the *real* invariant: the chosen text colour
must produce the higher contrast of the two options, regardless of
whether that contrast clears any absolute threshold.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
JohnMcLear added a commit to JohnMcLear/etherpad that referenced this pull request Apr 19, 2026
First cut of textColorFromBackgroundColor computed contrast against
pure black (L=0) and pure white (L=1), then returned the concrete
ether#222/#fff the pad actually renders with. For some mid-saturation
backgrounds the two comparisons disagreed — e.g. #ff0000:
  vs pure black = 5.25 → pick black → render ether#222 → actual 3.98
  vs pure white = 4.00 → would-render #fff → actual 4.00
The helper picked the wrong option because it compared against the
wrong target. Compare against the actual rendered colours so the
returned text colour is genuinely the higher-contrast choice.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
JohnMcLear added a commit to JohnMcLear/etherpad that referenced this pull request Apr 19, 2026
First cut of textColorFromBackgroundColor computed contrast against
pure black (L=0) and pure white (L=1), then returned the concrete
ether#222/#fff the pad actually renders with. For some mid-saturation
backgrounds the two comparisons disagreed — e.g. #ff0000:
  vs pure black = 5.25 → pick black → render ether#222 → actual 3.98
  vs pure white = 4.00 → would-render #fff → actual 4.00
The helper picked the wrong option because it compared against the
wrong target. Compare against the actual rendered colours so the
returned text colour is genuinely the higher-contrast choice.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants