Summary: See <https://phabricator.wikimedia.org/T906>. This behavior is a bug; we should remove the button if the user can't use the application.
Test Plan:
- With Macro uninstalled, did these things verifying the button vanished:
- Sent a user a message.
- Edited a revision.
- Edited repository basic information.
- Edited an initiative.
- Edited a Harbormaster build step.
- Added task comments.
- Edited profile blurb.
- Edited blog description.
- Commented on Pholio mock.
- Uploaded Pholio image.
- Edited Phortune merchant.
- Edited Phriction document.
- Edited Ponder answer.
- Edited Ponder question.
- Edited Slowvote poll.
- Edited a comment.
- Reinstalled Macro and saw button come back.
- Used button to put silly text on a funny picture.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D10900
Summary: I'm pretty sure that `@group` annotations are useless now... see D9855. Also fixed various other minor issues.
Test Plan: Eye-ball it.
Reviewers: #blessed_reviewers, epriestley, chad
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, Korvin, hach-que
Differential Revision: https://secure.phabricator.com/D9859
Summary: Did a more exhaustive grep on setIcon and found 99.9% of the icons.
Test Plan: I verified icon names on UIExamples, but unable to test some of the more complex flows visually. Mostly a read and replace.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin, hach-que
Differential Revision: https://secure.phabricator.com/D9088
Summary: The removes the sprite sheet 'icons' and replaces it with FontAwesome fonts.
Test Plan:
- Grep for SPRITE_ICONS and replace
- Grep for sprite-icons and replace
- Grep for PhabricatorActionList and choose all new icons
- Grep for Crumbs and fix icons
- Test/Replace PHUIList Icon support
- Test/Replace ObjectList Icon support (foot, epoch, etc)
- Browse as many pages as I could get to
- Remove sprite-icons and move remarkup to own sheet
- Review this diff in Differential
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin, hach-que
Differential Revision: https://secure.phabricator.com/D9052
Summary: A user in IRC hit this; this looks like a typo.
Test Plan: Created a new blog with a public view policy, a non-public join policy, and a custom domain.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: legien, epriestley
Differential Revision: https://secure.phabricator.com/D8980
Summary:
Fixes T4880. More specifically
- adds an "edit" pencil to post lists iff you can edit the post
- style change so this has no text-decoration
- adds a "no data" box if you have no posts in a given view
- style change to crush some margins so it formats like posts do
- adds some validation that your configuration is correct if you are specifying a custom domain
- updates docs about custom domains
Test Plan: clicked around and it was better! (see screenshots) read doc changes carefully
Reviewers: epriestley, chad
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T4880
Differential Revision: https://secure.phabricator.com/D8918
Summary:
nothing too crazy here. try to be smart about some defaults (i.e. phame title is optional and can be derived from title; post as not a draft by default; etc). Fixes T3695.
also do a little re-factoring to centralizing initializing new posts and turning posts into dictionaries. also change blogs => posts in another conduit method so it makes sense and stuff.
Test Plan: made some posts via conduit. testing trying to specify blogger, phame title, and isDraft, all worked nicely
Reviewers: chad, epriestley
Reviewed By: epriestley
Subscribers: aran, epriestley, Korvin
Maniphest Tasks: T3695
Differential Revision: https://secure.phabricator.com/D8485
Summary:
Via HackerOne. This defuses an attack which allows users to steal OAuth tokens through a clever sequence of steps:
- The attacker begins the OAuth workflow and copies the Facebook URL.
- The attacker mutates the URL to use the JS/anchor workflow, and to redirect to `/phame/live/X/` instead of `/login/facebook:facebook.com/`, where `X` is the ID of some blog they control. Facebook isn't strict about paths, so this is allowed.
- The blog has an external domain set (`blog.evil.com`), and the attacker controls that domain.
- The user gets stopped on the "live" controller with credentials in the page anchor (`#access_token=...`) and a message ("This blog has moved...") in a dialog. They click "Continue", which POSTs a CSRF token.
- When a user POSTs a `<form />` with no `action` attribute, the browser retains the page anchor. So visiting `/phame/live/8/#anchor` and clicking the "Continue" button POSTs you to a page with `#anchor` intact.
- Some browsers (including Firefox and Chrome) retain the anchor after a 302 redirect.
- The OAuth credentials are thus preserved when the user reaches `blog.evil.com`, and the attacker's site can read them.
This 302'ing after CSRF post is unusual in Phabricator and unique to Phame. It's not necessary -- instead, just use normal links, which drop anchors.
I'm going to pursue further steps to mitigate this class of attack more thoroughly:
- Ideally, we should render forms with an explicit `action` attribute, but this might be a lot of work. I might render them with `#` if no action is provided. We never expect anchors to survive POST, and it's surprising to me that they do.
- I'm going to blacklist OAuth parameters (like `access_token`) from appearing in GET on all pages except whitelisted pages (login pages). Although it's not important here, I think these could be captured from referrers in some cases. See also T4342.
Test Plan: Browsed all the affected Phame interfaces.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran, arice
Differential Revision: https://secure.phabricator.com/D8481
Summary:
Via HackerOne. We're missing this permissions check, so you can sneak around it with URL editing right now.
I checked the other queries in this application and they seem OK.
Test Plan: Tried to post to a blog I had no permission to join.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D8423
Summary: This removes the bulk of the "Form Errors" text, some variations likely exists. These are a bit redundant and space consuming. I'd also like to back ErrorView more into PHUIObjectBox.
Test Plan: Test out the forms, see errors without the text.
Reviewers: epriestley, btrahan
CC: Korvin, epriestley, aran, hach-que
Differential Revision: https://secure.phabricator.com/D7924
Summary:
Ref T4222. This fixes some issues with Phame's resource construction.
Phame requires a fully virtual resource source, and since I want to run wordpress templates unmodified some day I don't want to build resource maps for skins.
Move all the stuff that depends on resource lists being discoverable at build time to `CelerityPhysicalResources`, and only generate maps for subclasses.
The root `CelerityResources` can now construct virtual resources; construct a virtual resource for Phame and use it.
Test Plan: Off-domain blogs work correctly now. On-domain blogs with custom skins work correctly now.
Reviewers: btrahan, hach-que
Reviewed By: hach-que
CC: aran
Maniphest Tasks: T4222
Differential Revision: https://secure.phabricator.com/D7873
Summary:
Ref T4222.
- Removes the old map and changes the CelerityResourceMap API to be entirely driven by the new map.
- The new map is about 50% smaller and organized more sensibly.
- This removes the `/pkg/` URI component. All resources are now required to have unique names, so we can tell if a resource is a package or not by looking at the name.
- Removes some junky old APIs.
- Cleans up some other APIs.
- Added some feedback for `bin/celerity map`.
- `CelerityResourceMap` is still a singleton which is inextricably bound to the Phabricator map; this will change in the future.
Test Plan:
- Reloaded pages.
- Verified packaging works by looking at generated includes.
- Forced minification on and verified it worked.
- Forced no-timestamps on and verified it worked.
- Rebuilt map.
- Ran old script and verified error message.
- Checked logs.
Reviewers: btrahan, hach-que
Reviewed By: hach-que
CC: chad, aran
Maniphest Tasks: T4222
Differential Revision: https://secure.phabricator.com/D7872
Summary: We currently have a lot of calls to `addCrumb(id(new PhabricatorCrumbView())->...)` which can be expressed much more simply with a convenience method. Nearly all crumbs are only textual.
Test Plan:
- This was mostly automated, then I cleaned up a few unusual sites manually.
- Bunch of grep / randomly clicking around.
Reviewers: btrahan, chad
Reviewed By: btrahan
CC: hach-que, aran
Differential Revision: https://secure.phabricator.com/D7787
Summary: This builds out and implements PHUIPropertyListView (container) and PHUIPropertyListItemView (section) as well as adding tabs.
Test Plan: Tested each page I edited with the exception of Releeph and Phortune, though those changes look ok to me diff wise. Updated examples page with tabs.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: Korvin, epriestley, aran
Differential Revision: https://secure.phabricator.com/D7283
Summary:
Three changes here.
- Add `setActionList()`, and use that to set the action list.
- Add `setPropertyList()`, and use that to set the property list.
These will let us add some apropriate CSS so we can fix the border issue, and get rid of a bunch of goofy `.x + .y` selectors.
- Replace `addContent()` with `appendChild()`.
This is just a consistency thing; `AphrontView` already provides `appendChild()`, and `addContent()` did the same thing.
Test Plan:
- Viewed "All Config".
- Viewed a countdown.
- Viewed a revision (add comment, change list, table of contents, comment, local commits, open revisions affecting these files, update history).
- Viewed Diffusion (browse, change, history, repository, lint).
- Viewed Drydock (resource, lease).
- Viewed Files.
- Viewed Herald.
- Viewed Legalpad.
- Viewed macro (edit, edit audio, view).
- Viewed Maniphest.
- Viewed Applications.
- Viewed Paste.
- Viewed People.
- Viewed Phulux.
- Viewed Pholio.
- Viewed Phame (blog, post).
- Viewed Phortune (account, product).
- Viewed Ponder (questions, answers, comments).
- Viewed Releeph.
- Viewed Projects.
- Viewed Slowvote.
NOTE: Images in Files aren't on a black background anymore -- I assume that's on purpose?
NOTE: Some jankiness in Phortune, I'll clean that up when I get back to it. Not related to this diff.
Reviewers: chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D7174
Summary: This adds the 'PHUIObjectBox' to nearly every place that should get it. I need to comb through Diffusion a little more. I've left Differential mostly alone, but may decide to do it anyways this weekend. I'm sure I missed something else, but these are easy enough to update.
Test Plan: tested each new layout.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: Korvin, aran
Differential Revision: https://secure.phabricator.com/D7162
Summary: I'd like to reuse this for other content areas, renaming for now. This might be weird to keep setForm, but I can fix that later if we need.
Test Plan: reload a few forms in maniphest, projects, differential
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: Korvin, aran
Differential Revision: https://secure.phabricator.com/D7120
Summary: Instead of rendering this in all callers, just pass the object into the header and let it figure out how to format it.
Test Plan: Looked at Legalpad, Paste, and Pholio.
Reviewers: chad, btrahan
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D7039
Summary: Adds policy headers to more (all?) places currently in use.
Test Plan: test each page changed.
Reviewers: epriestley
Reviewed By: epriestley
CC: Korvin, aran
Differential Revision: https://secure.phabricator.com/D7032
Summary: The adds the ability to set 'properties' such as state, privacy, due date to the header of objects.
Test Plan: Implemented in Paste, Pholio. Tested various states.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: Korvin, aran
Differential Revision: https://secure.phabricator.com/D7016
Summary: Ref T603. Killing this class is cool because the classes that replace it are policy-aware. Tried to keep my wits about me as I did this and fixed a few random things along the way. (Ones I remember right now are pulling a query outside of a foreach loop in Releeph and fixing the text in UIExample to note that the ace of hearts if "a powerful" card and not the "most powerful" card (Q of spades gets that honor IMO))
Test Plan: tested the first few changes (execute, executeOne X handle, object) then got real mechanical / careful with the other changes.
Reviewers: epriestley
Reviewed By: epriestley
CC: Korvin, aran, FacebookPOC
Maniphest Tasks: T603
Differential Revision: https://secure.phabricator.com/D6941
Summary: Adds plain support for object lists that just look like lists
Test Plan: review UIexamples and a number of other applications
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: Korvin, aran
Differential Revision: https://secure.phabricator.com/D6922
Summary: Some more callsites, let me know if you see others, I think think is 98% of them now.
Test Plan: tested each page
Reviewers: epriestley, btrahan
Reviewed By: btrahan
CC: Korvin, aran
Differential Revision: https://secure.phabricator.com/D6814
Summary:
This attempts some consistency in form layouts. Notably, they all now contain headers and are 16px off the sides and tops of pages. Also updated dialogs to the same look and feel. I think I got 98% of forms with this pass, but it's likely I missed some buried somewhere.
TODO: will take another pass as consolidating these colors and new gradients in another diff.
Test Plan: Played in my sandbox all week. Please play with it too and let me know how they feel.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: Korvin, aran
Differential Revision: https://secure.phabricator.com/D6806
Summary:
^\s+(['"])dust\1\s*=>\s*true,?\s*$\n
Test Plan: Looked through the diff.
Reviewers: chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D6769
Summary:
Fixes T2691. Now, all PhabricatorActionListViews in the codebase setObjectHref to $request->getRequestURI. This value is passed over to PhabricatorActionItems right before they are rendered. If a PhabricatorActionItem is a workflow and there is no user OR the user is logged out, we used this objectURI to construct a log in URI.
Potentially added some undesirable behavior to aggressively setUser (and later setObjectURI) from within the List on Actions... This should be okay-ish unless there was a vision of actions having different user objects associated with them. I think this is a safe assumption.
Test Plan: played around with a mock all logged out (Ref T2652) and it worked!
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2691
Differential Revision: https://secure.phabricator.com/D6416
Summary: Currently you can't refresh the live blog or a blog post after clicking "View Live" due to POST action. I have removed the setRenderAsForm call on the "View Live" actions. I am unsure if this has any unintended consequences but I have tested and not found any.
Test Plan: Click the "View Live" action within a blog post or blog, and observe that the request occurs via GET not POST
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D6314
Summary:
This diff covers a bit of ground.
- PHUIDocumentExample has been added
- PHUIDocument has been extended with new features
- PhabricatorMenuView is now PHUIListView
- PhabricatorMenuItemView is now PHUIItemListView
Overall - I think I've gotten all the edges covered here. There is some derpi-ness that we can talk about, comments in the code. Responsive design is missing from the new features on PHUIDocument, will follow up later.
Test Plan: Tested mobile and desktop menus, old phriction layout, new document views, new lists, and object lists.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D6130
Summary: This swaps out ObjectItemListView for PHUIFeedStory when viewing posts in a Phame blog.
Test Plan: Write blog posts, published or not, and test in Phame. Web and iOS tested.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5989
Summary:
Ref T3023
Token support for Phriction Documents, Ponder Questions, and Phame Blogs
Test Plan: Token notifications and visual display seems to be working for the above types
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin, AnhNhan
Maniphest Tasks: T3023
Differential Revision: https://secure.phabricator.com/D5862
Summary:
This creates a common form look and feel across the site. I spent a bit of time working out a number of kinks in our various renderings. Some things:
- Font Styles are correctly applied for form elements now.
- Everything lines up!
- Selects are larger, easier to read, interact.
- Inputs have been squared.
- Consistant CSS applied glow (try it!)
- Improved Mobile Responsiveness
- CSS applied to all form elements, not just Aphront
- Many other minor tweaks.
I tried to hit as many high profile forms as possible in an effort to increase consistency. Stopped for now and will follow up after this lands. I know Evan is not a super fan of the glow, but after working with it for a week, it's way cleaner and responsive than the OS controls. Give it a try.
Test Plan: Tested many applications, forms, mobile and tablet.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5860
Summary: we need a link with type=self to pass xml validation. Note we will always fail validation until we remove any sigil-related stuff from the resultant HTML since we just made that up for our purposes. Also serves up the feed link on the blog view controller, complete with tooltip about how the custom domain stuff isn't supported.
Test Plan: viewed a blog page and liked the feed uri property. viewed a feed and verified the new link self stuff. post push I'll try the validator again.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin, vrana
Differential Revision: https://secure.phabricator.com/D5667
Summary: we don't post enough so this got reported as broken. Instead of specifying a published after date, just let the query class fetch the natural pagination limit ordered by id, and PHP will sort by publishedDate per usual. While its possible we might not get the right stuff relative to published date, its highly, highly unlikely given the large pagination size.
Test Plan: loaded up the blog xml link
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5593
Summary: Fished this out of the secure.phabricator.com error log. Introduced in D4869.
Test Plan: Eyeballed it.
Reviewers: chad, vrana
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D5199
Summary: Currently, you can set a blog URI to "evil.com" and then the live controller will issue a redirect. Instead, require a CSRF check. If it fails, pop a "this blog has moved" dialog.
Test Plan:
- Clicked "View Live" for in-app and on-domain blogs and posts.
- Hit URI directly.
{F33302}
Reviewers: vrana
Reviewed By: vrana
CC: cbg, aran
Differential Revision: https://secure.phabricator.com/D5021
Summary: Mostly applies a new call spacing rule; also a few things that have slipped through via pull requests and such
Test Plan: `find src/ -type f -name '*.php' | xargs -n16 arc lint --output summary --apply-patches`
Reviewers: chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D5002
Summary: Sgrepped for `"=~/</"` and manually changed every HTML.
Test Plan: This doesn't work yet but it is hopefully one of the last diffs before Phabricator will be undoubtedly HTML safe.
Reviewers: epriestley
CC: aran, Korvin
Maniphest Tasks: T2432
Differential Revision: https://secure.phabricator.com/D4927
Summary: I'm too lazy to attaching them for diffs where they were introduced.
Test Plan:
/
/D1, wrote comment with code snippet
DarkConsole
commit detail, wrote comment
task detail, wrote comment
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2432
Differential Revision: https://secure.phabricator.com/D4911
Summary:
Done by searching for `AphrontDialogView` and then `appendChild()`.
Also added some `pht()`.
Test Plan: None.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4882
