- Development - lists.drupal.org

[drupal-devel] [task] New layout module, looking for feedback
by FactoryJoe 31 Mar '05

31 Mar '05

Issue status update for http://drupal.org/node/18272 Project: Drupal Version: cvs Component: other Category: tasks Priority: normal Assigned to: Anonymous Reported by: nevets Updated by: FactoryJoe(a)civicspacelabs.org Status: active Can we see a sandbox version to play with? Or perhaps even screenshots? I'm working on this too and would like to see how you've approached the workflow. FactoryJoe(a)civicspacelabs.org Previous comments: ------------------------------------------------------------------------ March 2, 2005 - 12:20 : nevets Attachment: http://drupal.org/files/issues/layout.zip (16.93 KB) I have produced a new module for a site I am working on and would like to share it with the Drupal community. While not quit ready for release it is usable and I would appreciate feedback on how it fits into Drupal, what is extra and what might be added. An example of this in use can be found at http://test.wnpj.org (This is a test site) zip file attached with module, mysql file, default css file, support.inc and installation instructions. The module allows for breaking a page into sections similiar to the sections module but with a finer control on content. With it you can define one or more layout pages, each layout page is then referenced by the name layout/pagename (pagename is something you assign). For example, on the test site the layout page is called frontpage and under administer -> settings I have set Each layout page is broken into sections. A layout page can be layed out as either weighted rows (each section is one row in the output, order is determined by weight) or in a grid, placing each section in a row/column. In grid mode a section can span rows/columns. A section can have an optional "tab" which is used to label the section on output. Each section can be broken into layout blocks which are layed out in a row within the section. Each layout block can contain a 1) Drupal block (add block) 2) a Drupal node (add node) 3) a mixes list of Drupal nodes (add node) 4) a list of Drupal nodes of a given type. (add list) The difference between a "node" and a "list" is the degree of control you have. When adding a node you can specify a single node or list of nodes but the content is only changed by editing the list (or the node). With a list you can specify a table to join with (say event), specify the sort order and add a customer where clause. So with a list you can make a layout block that contains the list of events for the current week. So before looking at each of the parts in more detail, lets look at the common features. A layout page, section and block can be viewed by any one, only people not logged in, only people logged in or people with a specified permission. For a page, if it can not be views the user sees the "Access Denied" page. For layout sections and blocks no output is produced. For layout sections and blocks you can specify a ccs class that can be used to control the look of the section or block. So for example on test.wnpj.org, Alerts have a css class specified for the section to allow for a different color scheme than the rest. Details for layout pages, sections and blocks. Items with a * have a common footnote below on formatting control. Layout page Has a page name, used to access/display the page with the path layout/pagename Can specify a css file to include to control appearance of the page. Layout choice (weighted rows, grid) Permission controls Layout section *An optional "tab" header. Appearance controlled through css A description (defaults to tab header). Used in adminstative interface An enable flag (only enabled sections show in the output) Optional width (can be used to control the width of the section, also can be done is css) If weight row layout, a weight for the section. If grid layout, a row, column, row span and column span. (Note, if you change page layout type, existing data is retained so if you switch back the old values are used) Optional css class Permission controls Optional section header (if content). If specified and the section has content, placed above the layout blocks. Optional section header (if no content). If specified and the section does not have content, the specified html is used (Note: Can be used to make a section with fixed content like a welcome message without making a node) Optional section footer. If specified and the section has content, placed below the layout blocks. Hide if no content flag, if set and section has no content, "if not content header" is ignored and section produces no output. Layout block, common Description - Used in admin interface to identify block Weight - Determines order in section. Enable flag (only enabled blocks show in output) Optional width css class Layout block, type: Drupal block Source block - drop down list of available blocks Display control - Content of block only or title and content. Layout block, type: Node (or list of nodes) Node path list - a list of comma seperated nodes or path aliases (ex: 1,2,TaskForce,node/4) Display control Title Content Only Title and content Full Node (display by node type) Full Node with teaser (teaser instead of full body) *Title source, used if 'title' or 'title and content' picked for display, default @title@ *Content source, used if 'content' or 'title and content' picked for display, default @body@ Limit Content, used if 'content' or 'title and content' picked for display, default 0 (don't limit) If set, only used specified number of characters from content. Layout block, type: List Source module - used to select type of node that will be displayed Display control Title Content Only Title and content Full Node (display by node type) Full Node with teaser (teaser instead of full body) *Title source, used if 'title' or 'title and content' picked for display, default @title@ *Content source, used if 'content' or 'title and content' picked for display, default @body@ Limit Content, used if 'content' or 'title and content' picked for display, default 0 (don't limit) If set, only used specified number of characters from content. 'Sort by Sticky' flag, if set sticky items show at top of list. Ablity to select on node status, promote (frontpage) and sticky (each can be don't care, on, or off) WARNING - the following options require some internal knowledge of the selected source module Optional join table, example for the event module this would be event Allows limiting and sorting based on fields in the join table *Optional custom sort: example for event module: "start" Note: if 'sort by sticky' is set this appended to that. Optional where clause: see 'formatting' header for example Formatting tab header text, title, content and where clause The basic forms are text - just plain text, shows as entered @field_name@ - field in node table and for list in join table (does not apply to tab header) Ex: @title@ uses the title field of node {php code} - Output of php code is used These can be combined, here are some examples from test.wnpj.org event tab: Events for week of {layout_this_week_range(); } See support.inc for layout_this_week_range() event listing title: {event_format_date_range($node);}{event_city($node, true);} - @title@ event_format_date_range() and event_city() are custom functions. Note: There is a variable $node is scope at the time the function is called This contains the current node ( and for list the joined table ) event list where clause: {layout_sql_this_week();} See support.inc for layout_sql_this_week() Special note: By the time formatting happens title has been modified to be inclosed in a link to the actual node. Support.inc (preliminary) Contains functions that can be used in formatting layout_this_week($dow=1, $week_offset=0, $format = "") dow - dow of week to use for date, defaults to monday week_offset - can get date for week relative to this week format - valid format to date function if set, layout_this_week returns formatted date string else layout_this_week returns a date layout_this_week_range($week_offset=0, $format = "") Returns a string in format "Start date - End Date" week_offset - can get date for week relative to this week format - If sepecified, start date and end date are formatted as specifed Otherwise if start and end date are in same year 'M j' is used the format Otherwise if in different years 'M j Y' is used for the format layout_sql_this_week($clip=false) clip - not yet implemented, purpose is to limit dates for today to end of week For use with event module, returns sql where clause that selected entries only for current week. layout_mixed_type($node) - EXAMPLE Shows how one could format the title based on node type. Output appearance Fair degree of control in ouput controlled by css and theme functions (Current version is preliminary and subject to change) Using: After installing the module and adding the tables to the database Enable the module under administer -> module This will add a new entry under administer called layout From layout you add layout pages, sections and blocks Once you add a page you can add sections. Once you add a section you can add blocks ------------------------------------------------------------------------ March 2, 2005 - 13:09 : Dublin Drupaller this looks superb. Have downloaded and installed the module...but am slight confused how to implement a simple test. Can you outline step by step how to create a layout page test? Cheers Dub ------------------------------------------------------------------------ March 2, 2005 - 14:42 : nevets How to setup a simple test. First using just some nodes and blocks. You can either use existing nodes or make some test ones, you need to know either their node id or path alias (if path is enabled) Lets say the nodes are number 10, 12 and 23 (Your nodes are likely to be different) Go to administer -> layout Click on 'add layout page', give the page a name (probably should not have spaces) and set the other options you want. Save the layout page. Now you can add a section to to the page. Lets give it a tab, say 'Test layout', Fill in the other values as you wish just make sure that enable is checked. Save the section Now you can add layout blocks to the section. Lets add a node, click 'add node' (to right of section description) Give it a descriptive name, select what you want to display (you can come back and change this later) Now set node path list, for now just type one of your node ids (or a path alias) For now leave the other options as their defaults Save the layout block. To test what you have so far, go to administer -> settings Record what the value is for 'default front page' so you can set it back later. Change the value to layout/pagename where pagename is the name you assigned to the layout page. Save the settiings and go to the front page. Your should see a single tabbed section with the text 'Test layout' or what ever you typed. The contents should be from the node you selected displayed by you choice when setting up the node. (Note the default tabs are pretty boring) Now if you go back to administer -> layout you can add new pages, new sections and/or new blocks. Lets add a block to the existing section, click 'add block' Give the block a descriptive name, pick a block to display (normally I would pick one related to the node). Set the weight negative to position the block to the left of the node content, positive to position it to the right (This assumes you left the weight for the node block at 0). Save the block. Now lets add another section and then add a node block This time instead of single node type a comma separated list (ex 10,23,12 or TaskForce,23,Members). Nodes will be listed in the order given. Save the node If you have layout set to 'weighted rows' for the page the order of the sections will be determined by the weight. Now lets add another section this time NOT setting the tab header and placing text in the 'Section header (if no content)' text box. If using weighted rows set weight to -10 and save the section. The section can now act as a welcome block on the page. For the node blocks you can play around with display option and see how they change the contents of the sections. That covers everything but the list blocks. A simple example if you have the event module enable is add a list block to a new section. Set source module to 'event' Set display to your choice Set join table to 'event' Set custom sort to 'start' Set custom where to '{sql_this_week();}' Save the list block, this should give you a listing of events sorted by date for the current week. If you keep 'sort by sticky' set and any events for the week have it set they will 'float' to the top of the listing. You can also play with the statat, promote and stick value options. While the list block is very flexable, it useful to know something about the details of the node type to take advantage of the flexability. Hope this helps. Steve Ringwood ------------------------------------------------------------------------ March 3, 2005 - 10:26 : nevets this_week_range(); s/b layout_this_week_range(); and sql_this_week(); s/b layout_sql_this_week(); (Sorry about that, I had cut and paste from an earlier installation) Steve Ringwood ------------------------------------------------------------------------ March 3, 2005 - 11:28 : Boris Mann nevets -- the description sounds very interesting. It would be great if you were to apply for a CVS-account [1] and check your code in. [1] http://www.drupal.org/cvs-account ------------------------------------------------------------------------ March 4, 2005 - 16:36 : Steven Doesn't this module do what collimator.module does? ------------------------------------------------------------------------ March 4, 2005 - 17:05 : steffen This is truly a promising contribution, nevets! 1) How do I make a page only show the title and the excerpt for the assigned node? 2) I specify one page with grids and add two sections. In one section I put a node, and in the other I put a list. In the node I have specified 1 row, 1 rowspan, 1 column and 3 colspans. In the list I have specified 3 rows, 1 rowspan, 3 columns and 1 colspan. The result is that I get one long list, with the node on the top, and the list vertically underneath. Shouldn't the result be one node on the top, and a list in a 3x3 grid underneath? ------------------------------------------------------------------------ March 4, 2005 - 19:20 : nevets If by 'How do I make a page only show the title and the excerpt for the assigned node?' when you say excerpt you mean teaser, set display type to 'title and content' and set content source to '@teaser@' For the second question and the section for the list you wrote: 'In the list I have specified 3 rows, 1 rowspan, 3 columns and 1 colspan.' and ask 'Shouldn't the result be one node on the top, and a list in a 3x3 grid underneath?' Only if you set rowspan to 3, the row variable specifies the starting row, the row span the number of rows. Note the end result will effectively be a grid with 2 rows (node section and list section) and one column since they both span 3 columns. ------------------------------------------------------------------------ March 4, 2005 - 19:26 : nevets Regarding the collimator module, though you could probably reproduce part of the behaviour with the layout module (it will not make the multiple page) they do serve a different purpose. The layout module will allow a more arbitrary layout of content in the page and allows for combining the output from a single node, list of different node types, lists of nodes of a given type and also the content of blocks, When listing nodes of a given type you have control on how to filter them (the where clause) and how to order them. ------------------------------------------------------------------------ March 4, 2005 - 23:42 : ramdak5000(a)www.drupal.org I haven't tested the module, but, going by your description, it sounds great and what many people have been asking for, including myself. Can the module reference the taxonomy? We had a discussion (http://drupal.org/node/16426) where we wanted a kind of an index page automatically set for a vocabulary and grids on that page to automatically pulled in and displayed content from the vocab terms (not on the left and right side blocks). On urbanvancouver.com, for example, that index page would be 'neighbourhoods', with grids or whatever to display direct links to content posted under each neighbourhood category. Addditionally, some of us suggested that the breadcrumb trail automatically reflect this hierarchical structure. ------------------------------------------------------------------------ March 7, 2005 - 17:59 : Fool2 Doing taxonomy might be possible with the table join function, but I haven't figured it out yet. My website is organized according to taxonomy, so this is a big thing. ------------------------------------------------------------------------ March 14, 2005 - 14:33 : steffen Another issue regarding this very promising module: When listing nodes from a taxonomy, the module doesn't separate between the languages. So that if you have nodes in three different languages, every node will be listed three times, in three different languages. ------------------------------------------------------------------------ March 15, 2005 - 10:37 : jasonwhat This does look great. I see how to set "events" as a list, but the, blog nodetype doesn't have its own list. How do I set blogs as the type for a list, i.e.-"Featured Blogs"? Controlling by taxonomy would be great also. ------------------------------------------------------------------------ March 15, 2005 - 11:37 : chx Fixed the various fields: title, category and version. ------------------------------------------------------------------------ March 18, 2005 - 09:56 : jasonwhat where is the "fixed" version? Does database need updated for this or just the .module file? thanks. ------------------------------------------------------------------------ March 31, 2005 - 06:05 : chx jason, I fixed the fields of the issue, not the module itself. ------------------------------------------------------------------------ March 31, 2005 - 09:33 : jasonwhat Does anybody have this working on their site? ------------------------------------------------------------------------ March 31, 2005 - 09:49 : leafish_dylan I'm interested in testing this, because it looks very interesting. Is there any chance of it being "fixed" to work with 4.6's new SQL functions? ------------------------------------------------------------------------ March 31, 2005 - 09:56 : leafish_dylan Sorry, I changed the title by mistake.

1 0

[drupal-devel] Question on taxonomy_form() function
by Scott Courtney 31 Mar '05

31 Mar '05

Good morning! I've read the API docs on this, and even looked at the function's source code. I understand how it works, but apparently not how to use it properly. :-) My new image_import module (a feature that I'm developing in cooperation with walkah's excellent work, and yes, I've emailed with him to discuss it first) has code that looks like this: $html = ''; $vocabs =& taxonomy_get_vocabularies(IMAGE_IMPORT_IMAGE_NODETYPE); // The above function orders by weight, so we don't have to sort $group = ''; foreach ($vocabs as $vocab) { $group .= taxonomy_form($vocab->vid, $_POST['edit']['taxonomy'],NULL); } $html .= form_group(t('Categories'),$group,t(' ... long help text here ...')); The constant IMAGE_IMPORT_IMAGE_NODETYPE is just the string 'image'. After submitting a form, the $_POST['edit']['taxonomy'] subarray looks like this: array(3) { [0]=> string(1) "1" [1]=> array(1) { [0]=> string(1) "3" } [2]=> array(1) { [0]=> string(1) "7" } } The three selected term IDs are "1", "3", and "7". My problem is that the second vocabulary (vid==2, tid=={3,7}) allows multiple selection, whereas the first vocabulary (vid==1, tid==1) does not. The way the underlying form_select() called by taxonomy_form() seems to work is that it uses a subarray for the field only if multiple selection is allowed. This puts the resulting tids at different levels in the array tree, and causes my $_POST['edit']['taxonomy'] to fail as a way to pass the existing value(s) to taxonomy_form() for a given vid. My first thought was to try using a $name parameter for taxonomy_form(), something akin to taxonomy_1 for vid 1, taxonomy_2 for vid 2, etc. That seems awfully clumsy. I also tried passing $name as an array-like string, e.g., $name=="[taxonomy][1]" for vid 1, etc. That failed syntactically. There isn't a good way to match up those sub-subarrays with the vid to which they point, so simple conditional logic inside my foreach{} loop is not going to be feasible if the site administrator has associated more than one multi-term- allowed vocabulary with images. There's probably a "right way" to do this, but I'll be damned if I see it. The only method I can figure out looks like a hideous kludge. Can someone suggest the correct approach to this, or point me to a module that does it for some example code? The only place I could think of that does this is in node.module, but the problem there is that is has a $node object and passes that to an entirely different function instead of taxonomy_form. My module needs to pick vocabulary terms in advance of the existence of a node object, then apply them to multiple nodes as those nodes are created, so I don't have a $node to pass as node.module and other node-type modules do. My module doesn't define any new node types, but just adds a new way to create nodes of an existing type. I'm probably going to feel stupid when I see the answer to this, but I'll risk it. Any suggestions? Is it worth considering a new core function like taxonomy_forms() that would accept a node type as its parameter and return the HTML for all of the vocabs associated with that node, in such a way that the resulting $_POST is easily parsed by vid to support previews? Thanks! Scott -- -----------------------+------------------------------------------------------ Scott Courtney | "I don't mind Microsoft making money. I mind them scott(a)4th.com | having a bad operating system." -- Linus Torvalds http://4th.com/ | ("The Rebel Code," NY Times, 21 February 1999) | PGP Public Key at http://4th.com/keys/scott.pubkey

2 8

RE: [drupal-devel] "My Projects" on drupal.org -- broken?
by Boerland Bert 31 Mar '05

31 Mar '05

-----Original Message----- From: drupal-devel-bounces(a)drupal.org on behalf of Carl McDade > "you own no projects" gets my vote because there is the likelyhood of > the person clicking on the link being a developer wanting to contribute. I would say, display a link "My own projects" when a user has projects and display the link "All projects" when a user has no "own" projects. In case usability experts disagree with having useless links, it may be best not to display the link at all for useris without projects.

1 0

[drupal-devel] "My Projects" on drupal.org -- broken?
by Scott Courtney 31 Mar '05

31 Mar '05

I am logged in as my username on drupal.org, but when I go to "My Projects" (http://drupal.org/project/user) what I see is a list of *all* projects. Is this feature b0rk3n, or am I doing something wrong or misunderstanding what it's for? Scott -- -----------------------+------------------------------------------------------ Scott Courtney | "I don't mind Microsoft making money. I mind them scott(a)4th.com | having a bad operating system." -- Linus Torvalds http://4th.com/ | ("The Rebel Code," NY Times, 21 February 1999) | PGP Public Key at http://4th.com/keys/scott.pubkey

4 8

[drupal-devel] [feature] Request New Password Security
by Jose A Reyero 31 Mar '05

31 Mar '05

Issue status update for http://drupal.org/node/18719 Project: Drupal Version: cvs Component: user.module Category: feature requests Priority: critical Assigned to: Anonymous Reported by: neofactor Updated by: Jose A Reyero Status: patch Attachment: http://drupal.org/files/issues/user_one_time_login_2.patch (12.77 KB) Ok, I've started with chx's reviewed patch, and done most of Dries suggestions Main changes are: ----------------------- - Rewording, variable names, path, etc... as pointed out by Dries. Now it's user/reset. And yes, it was not clean-URL safe; it is now. - Allowed also for admin account, I dont like it though, but seem to be the only one :( - Used also for user registration, but I also kept old style login/password, think it can be some use and anyway, it's up to the site admin to remove it from the welcome e-mail. However it is only for 'no admin approval', see below about this.... - Added back in the config option for time-out, though reworked completely for usability. Default is '1 day' which I think should be reasonable. However, there's no time out for firts time users. About this, read below, please... ----------------------- Using this for first time login has some problems when admin aproval is required, as the hashed pass depends on 'changed' timestamp, and it changes when the admin approves an account. Actually, there's no way to know -or I didn't find it- when it is the first login for an approved account. For no admin approval, I just used 'changed' == 'created' as a criterium. I think we should be separating this changed date and last login date creating a new field in the user table, but that maybe for a future patch. That would also increase security, as the Administrator's info would be more accurate, and 'last login date' could be shown when user logs in, which is also security+ Related with this I've experienced problems, as administrator, when editing user accounts, then last login info for the users gets lost -is reset-... About the config option for time out, though I agree that enforcing security can be good in general, this is one of this cases where more security means less usability. And that decission should be left to Site administrators -each site has its own security requirements-. So I think the target here should be a 'reasonable' default, and then some flexibility for site admins. IMHO this latest patch is quite well balanced. So... everybody happy? Jose A Reyero Previous comments: ------------------------------------------------------------------------ March 11, 2005 - 03:52 : neofactor Problem: Any user can force another user's password to change... simply by selecting "request new password" and putting in their username. The user gets an email with the new.. but this feels like a violation to the user... and a pain. Solution? If someone requests a new password... Don't blindly change it... send an email that says...."Is this a real request authorized by you? Click here to confirm otherwise disregard this message" Please consider this critical for user by-in to Drupal as a secure system. I appreciate your consideration. http://drupal.org/node/18689 ------------------------------------------------------------------------ March 11, 2005 - 05:05 : neofactor I added some code to prevent the admin account from being reset... Please add as a patch. if ($account->uid == 1 { unset($account); form_set_error('name', t('Sorry. The username %name is not allowed to be changed.', array('%name' => ''. $edit['name'] .''))); } // Just above this code on line 911: if ($account) { $from = variable_get('site_mail', ini_get('sendmail_from')); $pass = user_password(); ------------------------------------------------------------------------ March 11, 2005 - 05:08 : killes(a)www.drop.org Please only set to "patch" if you are actually submitting one. The solution you proposed is rather a workaround and will not be acceptable for core. ------------------------------------------------------------------------ March 11, 2005 - 09:21 : Deno I was in charge of a web site with >10000 paying customers for some time, and the "customers care" was constantly troubled with pasword change requests, until we switched to a system that worked in the following way: 1) "I forgot the password" function accepts either users email, or users login name, generates a random phrase, and adds an entry to "pasword_change_request" table. This entry consists of: - User ID (corresponding to email or login) - random phrase - datestamp User ID is unique key, and the entries in this table older than MAX_TIME are regularly purged by a cron job. 2) System sends an email with a link to "change forgotten password" page with this random phrase as argument to the user. That would be something like: https://drupal.org/user/cfp/d438rwiuodw894732ehdw 3) When user follows the link, "cfp" page checks if the random phrase exists in the table, allows the user to immediately change the password, and sends another email with "your password was changed on RIGHT_NOW ... " note to the user in question. - Password change automatically triggers deleting of the corresponding entry in pasword_change_request table. - Password change automatically logs-in the user as well. This way, users weren't bothered by accidental password changes, and they also understood the system better than the one with automatically generated passwords. For additional security, we also built some brakes that assured that the number of attempts to change the password, such as: - "An email with instructions was sent to you X minutes ago. In case you don't receive it within N hours, please contact the site administrator" - "According to our logs, you have visited the CFP page more than NMAX times within last 24 hours. For security reasons, you will be denied access to this page for the next 24 hours. Please contact the site administrator." hope this helps... ------------------------------------------------------------------------ March 30, 2005 - 21:01 : Jose A Reyero Attachment: http://drupal.org/files/issues/user_one_time_login.patch (6.52 KB) This patch for -cvs- addresses this issue. It generates an *only one use* URL to login into user account, instead of a new password. Then the user clicks on the URL, logs in, and may change his old password if desired. It is a only one use URL, and has some additional security measures, like a configurable time-out -can be confgured in admin/user/settings- and also, if somebody uses it or password is changed in the meanwhile, previously generated login URLs of this type won't work. About how it works, it creates a new url with: - user id, - timestamp - a hash of current timestamp, changed date for account, and the old password hashed With this patch, this wont work for admin accounts, which I think is better, you never know... It has a number of features like: - User can be ignoring these emails if somebody else is requesting access, a new password request wont change current password. However, the user will be warned when seeing the emails - It reveals no sensible information about the user account in the URL itself - the generated hash is not predictable at all if you dont know the old password, as it depends on user's old password - there's a configurable timeout - The URL's are only one use, as they depend on two timestamps, one of which will be changing in case this login is used -or in case user logs in for other means - Any 'new password' request is independent of others, so no one can be interfering with one user asking for his password back. Also doesn't require any new db field nor keeping track of requested passwords. ------------------------------------------------------------------------ March 30, 2005 - 21:41 : chx Very nice patch. Please consider for 4.6. ------------------------------------------------------------------------ March 30, 2005 - 23:16 : drumm Couldn't those nested if statements be made into one with a bit cleaner indentation? It looks like the page which the user arrives at after getting the new login information is their user page, not a page to set the password. The "Time out for password recovery e-mail" configuration is very unfriendly. It should be a drop down select of the most used human readable durations. Or we could just decide on one timeout and leave that hard coded in; who actually needs to use that setting? The url is quite long, what could be done to make it shorter? ------------------------------------------------------------------------ March 31, 2005 - 00:50 : Jose A Reyero /Couldn't those nested if statements be made into one with a bit cleaner indentation?/ Maybe, but I had to split that if conditions in two parts because the $account var didn't get the value before the rest of the conditions -latest two- where evaluated. Someone can help with PHP here? /It looks like the page which the user arrives at after getting the new login information is their user page, not a page to set the password. The "Time out for password recovery e-mail" configuration is very unfriendly. It should be a drop down select of the most used human readable durations. Or we could just decide on one timeout and leave that hard coded in; who actually needs to use that setting?/ Agreed, will be polishing this a little bit, just wating for some more suggestions... For the duration, I'll change to 'hours', ok? Just don't like to restrict options using unneccessary dropdowns, it's only intended for administrators anyway. Maybe I add also a '0' for unlimited. Hardcoding it, would need anyway to be mentioned in the module help, so it's about the same overhead, both for coding and for the admin interface, and harcoding in two places in code is not good anyway. And I like having options, am I the only one? / The url is quite long, what could be done to make it shorter?/ Well, the url has to be long, it has neccessary params and an MD5 hash, which yes, could be cut someway, but that adding inneccessary code and also reducing security -though half an MD5 hash may be secure enough- but I see no point in that. ------------------------------------------------------------------------ March 31, 2005 - 02:18 : FactoryJoe(a)civicspacelabs.org "Or we could just decide on one timeout and leave that hard coded in; who actually needs to use that setting? " +1 for making this *not* configurable! C'mon, Drupal should have standards about security -- and if best practices say that, say, 8 hours, is a good amount of time for that URL to be active, then do that. Cut out the confusion and extra brain juice it takes to decide... "Gee, is 4 hours better than 5?" Additionally, there's a simple way to make this workflow better for the user. If you visit an outdated change password URL, you should be told, "Sorry, but that link you followed to change your password has expired. If you would like to reset your password, please fill out this form: [form] If you did not request to change your password, please notify [administrator's name w/ PM link]". This way, if you missed your change password deadline, you can request your password again right from the timeout page. If you didn't request the change, it will be obvious that someone else was trying to change it, and then you can contact the administrator and find out who it was that was messin' with your account. ------------------------------------------------------------------------ March 31, 2005 - 06:55 : chx Attachment: http://drupal.org/files/issues/user_one_time_login_0.patch (4.72 KB) I cleaned up the patch a bit. I hope code style is OK. So may -- there are no checks against the arguments? There is no need. user_load uses %d for 'uid', so that's dealt with. Comparison by < and > means an implicit cast to integer, so $timestamp is also dealt with automatically. I do not think timeout needs to be configured. Please comment on not letting uid 1 using this one. I am not sure whether this is necessary. ------------------------------------------------------------------------ March 31, 2005 - 07:12 : chx Attachment: http://drupal.org/files/issues/user_one_time_login_1.patch (6.21 KB) Implemented Chris' idea, so if you use a one time URl which is expired, you are sent to user/password with a message to ask for another. I changed a few messages and deleted the "not for admin" feature. Do we really want one gazillion support requests "I locked myself out of my site, user/password tells me not for admin, what now"? ------------------------------------------------------------------------ March 31, 2005 - 08:24 : chx One more point on uid 1. Yes, uid 1 can reset his pwd over the database. But then we would need to lock out administer users privileged users. Then administer nodes, 'cos of XSS. Then administer comments... then administer filters... OK, this is pointless. If someone can eavesdrop on your emails, everything is lost, I think. This is not something Drupal shall address. Maybe we could employ a security question -- secret answer pair. ------------------------------------------------------------------------ March 31, 2005 - 11:52 : Bèr Kessels Eventhough I am a less-settings advocate, I think this must remain a setting :). Why? Sites on Drupal differ a lot, and have a very differnet userbase. I run very low level sites, where it sometimes takes days before someone will read his/her mail. But I also know sites (drupal.org) where days would certainly be too long. Sorry Chris et al, I +1 on configurable duration. (hell, we really need some about:config alike screen in drupal) ------------------------------------------------------------------------ March 31, 2005 - 12:46 : Dries IMO, that setting has nothing to do with a site being "low level". If you request a new password, you probably want to login ASAP. I vote against a configuration option. Either way, this patch needs work: 1. We don't glue words together (eg. $hashedpass, $currenttime, $loginurl). Similarly, I suggest to change 'user/newpassword' to 'user/reset' or 'user/request-password'. 2. The patch exposes technical details to the user; terminology like "one time login URL" is likely to confuse many users. 3. The login URL is not clean URL safe, I think. 4. Please rename the variable $pass1 in user_pass_rehash(). 5. The code comments are often inconsistent. Some end with a punctuation, some don't. Some span two lines, others don't. On a related note, can't we reuse this system for the initial password? That is, instead of mailing a password, we mail an URL and force the user to enter a password.

1 0

[drupal-devel] improving the book module?
by Anisa 31 Mar '05

31 Mar '05

Is there anyone interested in improving the book module and can help out codewise? There are some ideas being bounced around on the docs list, such as offline editing, editing multiple pages at once (moving, deleting, etc), reducing the number of steps it takes to edit etc. But without the code to back it up, people are really just fussing at each other. ^.^; Anisa.

2 2

[drupal-devel] [feature] Request New Password Security
by Dries 31 Mar '05

31 Mar '05

Issue status update for http://drupal.org/node/18719 Project: Drupal Version: cvs Component: user.module Category: feature requests Priority: critical Assigned to: Anonymous Reported by: neofactor Updated by: Dries Status: patch IMO, that setting has nothing to do with a site being "low level". If you request a new password, you probably want to login ASAP. I vote against a configuration option. Either way, this patch needs work: 1. We don't glue words together (eg. $hashedpass, $currenttime, $loginurl). Similarly, I suggest to change 'user/newpassword' to 'user/reset' or 'user/request-password'. 2. The patch exposes technical details to the user; terminology like "one time login URL" is likely to confuse many users. 3. The login URL is not clean URL safe, I think. 4. Please rename the variable $pass1 in user_pass_rehash(). 5. The code comments are often inconsistent. Some end with a punctuation, some don't. Some span two lines, others don't. On a related note, can't we reuse this system for the initial password? That is, instead of mailing a password, we mail an URL and force the user to enter a password. Dries Previous comments: ------------------------------------------------------------------------ March 11, 2005 - 04:52 : neofactor Problem: Any user can force another user's password to change... simply by selecting "request new password" and putting in their username. The user gets an email with the new.. but this feels like a violation to the user... and a pain. Solution? If someone requests a new password... Don't blindly change it... send an email that says...."Is this a real request authorized by you? Click here to confirm otherwise disregard this message" Please consider this critical for user by-in to Drupal as a secure system. I appreciate your consideration. http://drupal.org/node/18689 ------------------------------------------------------------------------ March 11, 2005 - 06:05 : neofactor I added some code to prevent the admin account from being reset... Please add as a patch. if ($account->uid == 1 { unset($account); form_set_error('name', t('Sorry. The username %name is not allowed to be changed.', array('%name' => ''. $edit['name'] .''))); } // Just above this code on line 911: if ($account) { $from = variable_get('site_mail', ini_get('sendmail_from')); $pass = user_password(); ------------------------------------------------------------------------ March 11, 2005 - 06:08 : killes(a)www.drop.org Please only set to "patch" if you are actually submitting one. The solution you proposed is rather a workaround and will not be acceptable for core. ------------------------------------------------------------------------ March 11, 2005 - 10:21 : Deno I was in charge of a web site with >10000 paying customers for some time, and the "customers care" was constantly troubled with pasword change requests, until we switched to a system that worked in the following way: 1) "I forgot the password" function accepts either users email, or users login name, generates a random phrase, and adds an entry to "pasword_change_request" table. This entry consists of: - User ID (corresponding to email or login) - random phrase - datestamp User ID is unique key, and the entries in this table older than MAX_TIME are regularly purged by a cron job. 2) System sends an email with a link to "change forgotten password" page with this random phrase as argument to the user. That would be something like: https://drupal.org/user/cfp/d438rwiuodw894732ehdw 3) When user follows the link, "cfp" page checks if the random phrase exists in the table, allows the user to immediately change the password, and sends another email with "your password was changed on RIGHT_NOW ... " note to the user in question. - Password change automatically triggers deleting of the corresponding entry in pasword_change_request table. - Password change automatically logs-in the user as well. This way, users weren't bothered by accidental password changes, and they also understood the system better than the one with automatically generated passwords. For additional security, we also built some brakes that assured that the number of attempts to change the password, such as: - "An email with instructions was sent to you X minutes ago. In case you don't receive it within N hours, please contact the site administrator" - "According to our logs, you have visited the CFP page more than NMAX times within last 24 hours. For security reasons, you will be denied access to this page for the next 24 hours. Please contact the site administrator." hope this helps... ------------------------------------------------------------------------ March 30, 2005 - 22:01 : Jose A Reyero Attachment: http://drupal.org/files/issues/user_one_time_login.patch (6.52 KB) This patch for -cvs- addresses this issue. It generates an *only one use* URL to login into user account, instead of a new password. Then the user clicks on the URL, logs in, and may change his old password if desired. It is a only one use URL, and has some additional security measures, like a configurable time-out -can be confgured in admin/user/settings- and also, if somebody uses it or password is changed in the meanwhile, previously generated login URLs of this type won't work. About how it works, it creates a new url with: - user id, - timestamp - a hash of current timestamp, changed date for account, and the old password hashed With this patch, this wont work for admin accounts, which I think is better, you never know... It has a number of features like: - User can be ignoring these emails if somebody else is requesting access, a new password request wont change current password. However, the user will be warned when seeing the emails - It reveals no sensible information about the user account in the URL itself - the generated hash is not predictable at all if you dont know the old password, as it depends on user's old password - there's a configurable timeout - The URL's are only one use, as they depend on two timestamps, one of which will be changing in case this login is used -or in case user logs in for other means - Any 'new password' request is independent of others, so no one can be interfering with one user asking for his password back. Also doesn't require any new db field nor keeping track of requested passwords. ------------------------------------------------------------------------ March 30, 2005 - 22:41 : chx Very nice patch. Please consider for 4.6. ------------------------------------------------------------------------ March 31, 2005 - 00:16 : drumm Couldn't those nested if statements be made into one with a bit cleaner indentation? It looks like the page which the user arrives at after getting the new login information is their user page, not a page to set the password. The "Time out for password recovery e-mail" configuration is very unfriendly. It should be a drop down select of the most used human readable durations. Or we could just decide on one timeout and leave that hard coded in; who actually needs to use that setting? The url is quite long, what could be done to make it shorter? ------------------------------------------------------------------------ March 31, 2005 - 01:50 : Jose A Reyero /Couldn't those nested if statements be made into one with a bit cleaner indentation?/ Maybe, but I had to split that if conditions in two parts because the $account var didn't get the value before the rest of the conditions -latest two- where evaluated. Someone can help with PHP here? /It looks like the page which the user arrives at after getting the new login information is their user page, not a page to set the password. The "Time out for password recovery e-mail" configuration is very unfriendly. It should be a drop down select of the most used human readable durations. Or we could just decide on one timeout and leave that hard coded in; who actually needs to use that setting?/ Agreed, will be polishing this a little bit, just wating for some more suggestions... For the duration, I'll change to 'hours', ok? Just don't like to restrict options using unneccessary dropdowns, it's only intended for administrators anyway. Maybe I add also a '0' for unlimited. Hardcoding it, would need anyway to be mentioned in the module help, so it's about the same overhead, both for coding and for the admin interface, and harcoding in two places in code is not good anyway. And I like having options, am I the only one? / The url is quite long, what could be done to make it shorter?/ Well, the url has to be long, it has neccessary params and an MD5 hash, which yes, could be cut someway, but that adding inneccessary code and also reducing security -though half an MD5 hash may be secure enough- but I see no point in that. ------------------------------------------------------------------------ March 31, 2005 - 03:18 : FactoryJoe(a)civicspacelabs.org "Or we could just decide on one timeout and leave that hard coded in; who actually needs to use that setting? " +1 for making this *not* configurable! C'mon, Drupal should have standards about security -- and if best practices say that, say, 8 hours, is a good amount of time for that URL to be active, then do that. Cut out the confusion and extra brain juice it takes to decide... "Gee, is 4 hours better than 5?" Additionally, there's a simple way to make this workflow better for the user. If you visit an outdated change password URL, you should be told, "Sorry, but that link you followed to change your password has expired. If you would like to reset your password, please fill out this form: [form] If you did not request to change your password, please notify [administrator's name w/ PM link]". This way, if you missed your change password deadline, you can request your password again right from the timeout page. If you didn't request the change, it will be obvious that someone else was trying to change it, and then you can contact the administrator and find out who it was that was messin' with your account. ------------------------------------------------------------------------ March 31, 2005 - 07:55 : chx Attachment: http://drupal.org/files/issues/user_one_time_login_0.patch (4.72 KB) I cleaned up the patch a bit. I hope code style is OK. So may -- there are no checks against the arguments? There is no need. user_load uses %d for 'uid', so that's dealt with. Comparison by < and > means an implicit cast to integer, so $timestamp is also dealt with automatically. I do not think timeout needs to be configured. Please comment on not letting uid 1 using this one. I am not sure whether this is necessary. ------------------------------------------------------------------------ March 31, 2005 - 08:12 : chx Attachment: http://drupal.org/files/issues/user_one_time_login_1.patch (6.21 KB) Implemented Chris' idea, so if you use a one time URl which is expired, you are sent to user/password with a message to ask for another. I changed a few messages and deleted the "not for admin" feature. Do we really want one gazillion support requests "I locked myself out of my site, user/password tells me not for admin, what now"? ------------------------------------------------------------------------ March 31, 2005 - 09:24 : chx One more point on uid 1. Yes, uid 1 can reset his pwd over the database. But then we would need to lock out administer users privileged users. Then administer nodes, 'cos of XSS. Then administer comments... then administer filters... OK, this is pointless. If someone can eavesdrop on your emails, everything is lost, I think. This is not something Drupal shall address. Maybe we could employ a security question -- secret answer pair. ------------------------------------------------------------------------ March 31, 2005 - 12:52 : Bèr Kessels Eventhough I am a less-settings advocate, I think this must remain a setting :). Why? Sites on Drupal differ a lot, and have a very differnet userbase. I run very low level sites, where it sometimes takes days before someone will read his/her mail. But I also know sites (drupal.org) where days would certainly be too long. Sorry Chris et al, I +1 on configurable duration. (hell, we really need some about:config alike screen in drupal)

1 0

[drupal-devel] [feature] Request New Password Security
by Bèr Kessels 31 Mar '05

31 Mar '05

Issue status update for http://drupal.org/node/18719 Project: Drupal Version: cvs Component: user.module Category: feature requests Priority: critical Assigned to: Anonymous Reported by: neofactor Updated by: Bèr Kessels Status: patch Eventhough I am a less-settings advocate, I think this must remain a setting :). Why? Sites on Drupal differ a lot, and have a very differnet userbase. I run very low level sites, where it sometimes takes days before someone will read his/her mail. But I also know sites (drupal.org) where days would certainly be too long. Sorry Chris et al, I +1 on configurable duration. (hell, we really need some about:config alike screen in drupal) Bèr Kessels Previous comments: ------------------------------------------------------------------------ March 11, 2005 - 03:52 : neofactor Problem: Any user can force another user's password to change... simply by selecting "request new password" and putting in their username. The user gets an email with the new.. but this feels like a violation to the user... and a pain. Solution? If someone requests a new password... Don't blindly change it... send an email that says...."Is this a real request authorized by you? Click here to confirm otherwise disregard this message" Please consider this critical for user by-in to Drupal as a secure system. I appreciate your consideration. http://drupal.org/node/18689 ------------------------------------------------------------------------ March 11, 2005 - 05:05 : neofactor I added some code to prevent the admin account from being reset... Please add as a patch. if ($account->uid == 1 { unset($account); form_set_error('name', t('Sorry. The username %name is not allowed to be changed.', array('%name' => ''. $edit['name'] .''))); } // Just above this code on line 911: if ($account) { $from = variable_get('site_mail', ini_get('sendmail_from')); $pass = user_password(); ------------------------------------------------------------------------ March 11, 2005 - 05:08 : killes(a)www.drop.org Please only set to "patch" if you are actually submitting one. The solution you proposed is rather a workaround and will not be acceptable for core. ------------------------------------------------------------------------ March 11, 2005 - 09:21 : Deno I was in charge of a web site with >10000 paying customers for some time, and the "customers care" was constantly troubled with pasword change requests, until we switched to a system that worked in the following way: 1) "I forgot the password" function accepts either users email, or users login name, generates a random phrase, and adds an entry to "pasword_change_request" table. This entry consists of: - User ID (corresponding to email or login) - random phrase - datestamp User ID is unique key, and the entries in this table older than MAX_TIME are regularly purged by a cron job. 2) System sends an email with a link to "change forgotten password" page with this random phrase as argument to the user. That would be something like: https://drupal.org/user/cfp/d438rwiuodw894732ehdw 3) When user follows the link, "cfp" page checks if the random phrase exists in the table, allows the user to immediately change the password, and sends another email with "your password was changed on RIGHT_NOW ... " note to the user in question. - Password change automatically triggers deleting of the corresponding entry in pasword_change_request table. - Password change automatically logs-in the user as well. This way, users weren't bothered by accidental password changes, and they also understood the system better than the one with automatically generated passwords. For additional security, we also built some brakes that assured that the number of attempts to change the password, such as: - "An email with instructions was sent to you X minutes ago. In case you don't receive it within N hours, please contact the site administrator" - "According to our logs, you have visited the CFP page more than NMAX times within last 24 hours. For security reasons, you will be denied access to this page for the next 24 hours. Please contact the site administrator." hope this helps... ------------------------------------------------------------------------ March 30, 2005 - 21:01 : Jose A Reyero Attachment: http://drupal.org/files/issues/user_one_time_login.patch (6.52 KB) This patch for -cvs- addresses this issue. It generates an *only one use* URL to login into user account, instead of a new password. Then the user clicks on the URL, logs in, and may change his old password if desired. It is a only one use URL, and has some additional security measures, like a configurable time-out -can be confgured in admin/user/settings- and also, if somebody uses it or password is changed in the meanwhile, previously generated login URLs of this type won't work. About how it works, it creates a new url with: - user id, - timestamp - a hash of current timestamp, changed date for account, and the old password hashed With this patch, this wont work for admin accounts, which I think is better, you never know... It has a number of features like: - User can be ignoring these emails if somebody else is requesting access, a new password request wont change current password. However, the user will be warned when seeing the emails - It reveals no sensible information about the user account in the URL itself - the generated hash is not predictable at all if you dont know the old password, as it depends on user's old password - there's a configurable timeout - The URL's are only one use, as they depend on two timestamps, one of which will be changing in case this login is used -or in case user logs in for other means - Any 'new password' request is independent of others, so no one can be interfering with one user asking for his password back. Also doesn't require any new db field nor keeping track of requested passwords. ------------------------------------------------------------------------ March 30, 2005 - 21:41 : chx Very nice patch. Please consider for 4.6. ------------------------------------------------------------------------ March 30, 2005 - 23:16 : drumm Couldn't those nested if statements be made into one with a bit cleaner indentation? It looks like the page which the user arrives at after getting the new login information is their user page, not a page to set the password. The "Time out for password recovery e-mail" configuration is very unfriendly. It should be a drop down select of the most used human readable durations. Or we could just decide on one timeout and leave that hard coded in; who actually needs to use that setting? The url is quite long, what could be done to make it shorter? ------------------------------------------------------------------------ March 31, 2005 - 00:50 : Jose A Reyero /Couldn't those nested if statements be made into one with a bit cleaner indentation?/ Maybe, but I had to split that if conditions in two parts because the $account var didn't get the value before the rest of the conditions -latest two- where evaluated. Someone can help with PHP here? /It looks like the page which the user arrives at after getting the new login information is their user page, not a page to set the password. The "Time out for password recovery e-mail" configuration is very unfriendly. It should be a drop down select of the most used human readable durations. Or we could just decide on one timeout and leave that hard coded in; who actually needs to use that setting?/ Agreed, will be polishing this a little bit, just wating for some more suggestions... For the duration, I'll change to 'hours', ok? Just don't like to restrict options using unneccessary dropdowns, it's only intended for administrators anyway. Maybe I add also a '0' for unlimited. Hardcoding it, would need anyway to be mentioned in the module help, so it's about the same overhead, both for coding and for the admin interface, and harcoding in two places in code is not good anyway. And I like having options, am I the only one? / The url is quite long, what could be done to make it shorter?/ Well, the url has to be long, it has neccessary params and an MD5 hash, which yes, could be cut someway, but that adding inneccessary code and also reducing security -though half an MD5 hash may be secure enough- but I see no point in that. ------------------------------------------------------------------------ March 31, 2005 - 02:18 : FactoryJoe(a)civicspacelabs.org "Or we could just decide on one timeout and leave that hard coded in; who actually needs to use that setting? " +1 for making this *not* configurable! C'mon, Drupal should have standards about security -- and if best practices say that, say, 8 hours, is a good amount of time for that URL to be active, then do that. Cut out the confusion and extra brain juice it takes to decide... "Gee, is 4 hours better than 5?" Additionally, there's a simple way to make this workflow better for the user. If you visit an outdated change password URL, you should be told, "Sorry, but that link you followed to change your password has expired. If you would like to reset your password, please fill out this form: [form] If you did not request to change your password, please notify [administrator's name w/ PM link]". This way, if you missed your change password deadline, you can request your password again right from the timeout page. If you didn't request the change, it will be obvious that someone else was trying to change it, and then you can contact the administrator and find out who it was that was messin' with your account. ------------------------------------------------------------------------ March 31, 2005 - 06:55 : chx Attachment: http://drupal.org/files/issues/user_one_time_login_0.patch (4.72 KB) I cleaned up the patch a bit. I hope code style is OK. So may -- there are no checks against the arguments? There is no need. user_load uses %d for 'uid', so that's dealt with. Comparison by < and > means an implicit cast to integer, so $timestamp is also dealt with automatically. I do not think timeout needs to be configured. Please comment on not letting uid 1 using this one. I am not sure whether this is necessary. ------------------------------------------------------------------------ March 31, 2005 - 07:12 : chx Attachment: http://drupal.org/files/issues/user_one_time_login_1.patch (6.21 KB) Implemented Chris' idea, so if you use a one time URl which is expired, you are sent to user/password with a message to ask for another. I changed a few messages and deleted the "not for admin" feature. Do we really want one gazillion support requests "I locked myself out of my site, user/password tells me not for admin, what now"? ------------------------------------------------------------------------ March 31, 2005 - 08:24 : chx One more point on uid 1. Yes, uid 1 can reset his pwd over the database. But then we would need to lock out administer users privileged users. Then administer nodes, 'cos of XSS. Then administer comments... then administer filters... OK, this is pointless. If someone can eavesdrop on your emails, everything is lost, I think. This is not something Drupal shall address. Maybe we could employ a security question -- secret answer pair.

1 0

[drupal-devel] [task] Drupal Install System
by adrian 31 Mar '05

31 Mar '05

Issue status update for http://drupal.org/node/5496 Project: Drupal Version: cvs Component: base system Category: tasks Priority: normal Assigned to: adrian Reported by: adrian Updated by: adrian Status: patch Yeah. I've been planning that from the start (before it had a fancy name), I'm currently planning to have it happen around phase four or five. adrian Previous comments: ------------------------------------------------------------------------ January 29, 2004 - 11:10 : adrian I am in the process of developing a new Install API for the drupal core, which will allow a completely web based install / set up process, as well as allow contributed modules to integrate more directly into this system. A more detailed description of the system is available in this thread. This task has been created to place each of the successive patches in the patch queue. ------------------------------------------------------------------------ January 29, 2004 - 11:20 : adrian Attachment: http://drupal.org/files/issues/install_api_first_level.patch (6.42 KB) Kjartan correctly identified one of the problems in creating a unified install system. There's a lot of code that gets run by simply including the core files (bootstrap.inc, database.inc, session.inc and common.inc). A lot of this code relies upon the database connection going off without a hitch, like for instance the current system variables, themes and session code. To work around this i created a new drupal_init() function, which needs to be called when you want the system to initialize. drupal_init has a 'db_init' argument, which allows you to bypass the loading of all the database related code with a simple flag. install.php needs to run drupal core, while ignoring any database connections .. until atleast the point where the configuration settings for the database are entered, and confirmed to work. There is a new function created called 'database_connected()' , which returns a boolean true only if the database both connected and the table was selected successfully. This patch does not have a suitable error message for index.php upon not being able to make a database connection ,but that message should very likely be discussed and finalized. In the next level of the patch I will introduce the major changes, or rather additions. I haven't modified any existing files beyond the ones in this patch (at this time). ------------------------------------------------------------------------ January 29, 2004 - 14:24 : Kjartan This patch has some serious flaws though. <?php include_once "includes/bootstrap.inc"; -drupal_page_header(); include_once "includes/common.inc"; ?> By removing the drupal_page_header() you break the whole point of bootstrap as the common.inc stuff is included when its not necessary. It needs some more thought. Maybe you should take the time to make a workflow of the current Drupal init process before modifying it. Also the naming of constants, I would prefer calling them DATABASE_ATTEMPT as it makes it easier to tell them appart. ------------------------------------------------------------------------ February 9, 2004 - 22:18 : adrian I have reworked the patch into a very minimal set of requirements. To prevent the .inc files from running any code when you don't want them to , you 'define' a constant called 'DRUPAL_NO_INIT' before including bootstrap.inc. This relatively small patch is required for the install stuff to work. ------------------------------------------------------------------------ February 9, 2004 - 22:22 : adrian Attachment: http://drupal.org/files/issues/drupal_no_init_flag.patch (3.88 KB) i'll even attach it this time. ------------------------------------------------------------------------ February 9, 2004 - 22:48 : adrian Attachment: http://drupal.org/files/issues/database_init_return.patch (1.96 KB) The following patch is a modification to the includes/database*.inc set of files, to make db_connect return a boolean value depending on wether the connection was successfully made. I have changed database.inc to die() upon connection error with the message "Database Connection Unsuccessful". Cleaning up that message and placing a help page might be a good idea from an user experience point of view. This patch is required for the install system, in that it needs to test wether it has made a successfull database connection before it allows you to actually install the schema. ------------------------------------------------------------------------ February 10, 2004 - 03:15 : adrian Attachment: http://drupal.org/files/issues/install_system_alpha.tar.bz2 (11.59 KB) This is an alpha version of the new install API code. It is completely capable of installing the drupal core system without requiring any interaction with the database side. There are currently 5 configuration screens , but the API is sound for us to add / remove / change these screens as we need. In it's current state, it isn't capable of updating the base system, however the backend stubs exist, and have been tested. In the push to get the install process running smoothly i simply disabled the update interface. The previous 2 patches on this node need to be applied, but have been combined into install_system_alpha.patch which is included in this distribution. The rest of the files are new , and should not interfere with the drupal core at all. They are includes/install.inc ... the file that hosts the install api. install.php ... the front end script that initializes the install api (a total of 5 lines) install/system.install ... the install module for the core. It contains the bulk of code from update.php, and all the configuration information needed to successfully install the core. In the future , the install api will be able to easily install any contrib module through the modulename.install file that will hopefully become well supported. Note: I need to update the postgres compatibility in this .install file, and as such only mysql is fully tested now. I have posted some screenshots at my site ------------------------------------------------------------------------ February 10, 2004 - 20:22 : adrian Unconed just pointed out there is a conflict with windows installations in that there can not be an 'INSTALL' file and an 'install' directory (which contains the system.install currently) He suggested that we rename the README and INSTALL files to README.txt and INSTALL.txt. I personally move that we store the .install files within the modules directory , as in the future installing a contributed module should be as simple as dropping the module directory (containing .install , .module and whichever external files) into modules/ , and then installing the module within drupal. ------------------------------------------------------------------------ February 12, 2004 - 07:10 : moshe weitzman I played around with the patch, and successfully installed a new Drupal instance on the fitst try. Nice work, Adrian! I reviewed the code too, and found it quite Drupalish and satsfactory. Here are some notes from my install: - On the configuration file page, I selected from the radio button options a file which was not conf.php and thus did not exist. I strugged with the 'file is not writeable' error for a while until I realized that you expected me to create the file and then the installer would edit it ... in general, this page is a pretty complex introduction into drupal. I suggest simply assuming 'conf.php' here. I haven't thought about this much though. - Similar to above, I figured Drupal would create a database for me. Not so, I had to create it and then Drupal takes over. Some simple help text adddresses this point. - Use form_password() or similar when requesting a database password - The default $base_url was missing a slall for me. It said 'http://medrinstall' instead of 'http://me/drinstall' - After completing the DB install, there is no page telling you what to do next. A link to the home page is all thats needed IMO. -The site_name chosen during the install was not saved in the 'variable' table. - When you return to install.php some time later, you have no indication what a working site already exists. Not sure thats the right thing to do. Not a big deal though. P.S. Adrians suggestion to store the .install files in module specific subdirs under modules makes sense to me. Thus system.install would move to modules/system/system.install ------------------------------------------------------------------------ February 13, 2004 - 06:04 : skip I had to set up a pMachine blog yesterday for a client. I have to say... it was the easiest install process I've ever gone through (CMS-wise). Very user friendly. Lots of feedback to the user. If you haven't seen it before, you should check it out for ideas. ------------------------------------------------------------------------ February 18, 2004 - 07:26 : adrian Attachment: http://drupal.org/files/issues/install_phase2.tar.bz2 (12.96 KB) So I have spent far too much time over the last few weeks battling with this install system. My current results are in this here tarball. This time I am even less sure of it fully working in all cases, than last time. With this release I added extensive error checking to the install process, finished MOST of what is needed for the update process , but most importantly... it is a lot more secure than previously, and is specifically built to allow people to host their multiple drupal installs from just a single root. I have handled this by adding a global variable to the config file (allow_override_config) , which is checked by the install script before a new configuration file is made that would override the currently active one. There are three settings to this : 'no' , 'any' and 'auth'. I feel auth is generally the best option, as it requires anyone trying to override a working configuration file to enter the username and password for the site he is trying to override. Added checking also exists for the minimum requirements of running a drupal system (the tests probably need to be fleshed out), aswell as refusing to install a database dump into a db_url/db_prefix which seems to have a valid site installed. When a working configuration is detected on startup , the default action is to require the administrator user of the site to log in. This is for security reasons. The install script has lots of code that stops it from ever overwriting any config file that has already been created, but as I said .. it allows for the creation of a config file that will be parsed before the current valid install, but only with the express permission of that config. Config files have been moved to './conf/' , and the default config has been renamed to 'conf/default.php'. I have split install.inc into install.inc and install.module, as there was a lot of interface code specifically for the update/install sequence (and it is best available for reuse .. so i didnt wanna put it in install.php). This has an unrelated quirk for xtemplate .. I left it in the patch because for some reason I was unable to get a working default theme because xtemplate refused to populate the 'path' template variable. I also have an extensive 'gallery' of the new changes if you just want to see what it looks like. Directions for use : untar in a drupal cvs tree , and 'patch -p0 < install_alpha_2.patch' go to http://mysite/install.php ... previous configs in includes/ will be ignored for extra fun , create a hostname entry to sitea.siteb.sitec.sited.mysite , and watch the install script pick up the valid configs. ------------------------------------------------------------------------ February 18, 2004 - 17:14 : TDobes Adrian, thanks for all your work on the install system... I'll check it out once I get home this evening. Just FYI, here's a link to the XTemplate bug [1] you mentioned. (It's a simple two-line patch.) [1] http://drupal.org/node/view/5858 ------------------------------------------------------------------------ June 20, 2004 - 22:29 : adrian taking out of patch queue. Still on my todo list, but I have to get some other things done first. ------------------------------------------------------------------------ July 27, 2004 - 17:25 : JonBob The install system should (will?) handle version numbers. Some related issues to examine: None of these are really duplicates of one another, so while I'd like to close them I can't. Consider this a reminder to do so if and when the install system addresses the problems. New 'version' system field and Drupal version [2] Module dependencies [3] Show drupal version to admins [4] Meta tag with drupal version [5] Version number [6] [2] http://drupal.org/node/view/5811 [3] http://drupal.org/node/view/8402 [4] http://drupal.org/node/view/2986 [5] http://drupal.org/node/view/2828 [6] http://drupal.org/node/view/8762 ------------------------------------------------------------------------ March 13, 2005 - 18:03 : publian It made it based on install_phase2. It makes it with drupal CVS as of 2005.03.14. The improvement point is as follows. 1) The installation query command execution read database/database.mysql or database/database.pgsql. 2) Two head bytes of $_SERVER 'HTTP_ACCEPT_LANGUAGE' were seen, and the installation language was distinguished automatically. This sees the install.lang file in the languege folder. (funcution lt() like funcution t()) Making) 3)　In the installation process, the mother tongue can have been selected. (The import is done from the file in the translations folder. ) 4) Additionally, a detailed place was corrected. The bug moves though thinks many still. -- Drupal Japan ------------------------------------------------------------------------ March 13, 2005 - 18:10 : publian Attachment: http://drupal.org/files/issues/install_phase3.tar.gz (134.72 KB) It is as for appending. ------------------------------------------------------------------------ March 26, 2005 - 10:11 : judah I was just about to start creating an install system for modules when someone mentioned you were already working on one. Here is my design specs, http://drupal.org/node/19427 I could not contact you via any other means so please advise what the current status is on this. Thanks ------------------------------------------------------------------------ March 31, 2005 - 01:13 : adrian Attachment: http://drupal.org/files/issues/install_api_phase_1.tar.bz2 (21.49 KB) Attached is the first iteration of my new install system. To install : Extract tarball into a clean checkout, and apply the install_api_phase_1.patch file. This patch implements the following : Core install api. /(includes/install.inc)/ Modifications to drupal_load that disables a module that has an out of data schema. /(includes/bootstrap.inc)/ Does not allow modules to be enabled unless they have been installed, or updated and the allow for them to be installed and updated directly from admin/modules. /(modules/system.module)/ This patch does NOT : Install or update Drupal core, however it is possible to do so with the api (and I have done so before), significant changes to Drupal need to happen before it is practical to handle core with it. Description : This is the first phase of a four or five phase project, so it's not meant to solve the whole problem. I am writing a post for the Drupal forums at present detailing the progress with the install system, and directions I want to take it. To enable a module to be installable, you add an implementation of hook_info() to it , i.e. : <?php function buddylist_info($key) { $info['schema'] = 1; return $info[$key]; } ?> This tells the system that a schema version of 1 is required to operate this module. You then create a file called modulename.install (preferably in the same directory in the module, although it doesn't really matter. You then need to specify which schema version it implements by : <?php function buddylist_schema($key) { $info['version'] = 1; return $info[$key]; } ?> Although, in the current implementation this function is optional when you only have a single schema version (the default is 1). Then you implement your hook_install(). <?php function buddylist_install() { $query = " CREATE TABLE {buddylist} ( uid int(10) UNSIGNED NOT NULL, buddy int(10) UNSIGNED NOT NULL, timestamp int(11) NOT NULL, PRIMARY KEY (uid, buddy) )"; db_query($query); db_query("ALTER TABLE {buddylist} ADD received tinyint(1) NOT NULL;"); } ?> This function would be called for any database type that has not specified it's own function. To add Postgres support, the function would be called buddylist_install_pgsql();. If the schema of your module ever changes, to allow for it to be updated, you proceed as follows : increment the 'schema' value in your hook_info() function. increment the 'version' value in your hook_schema function, within your modulename.install file. modify your schema as required, by changing it in the hook_install() function. provide an update script for users of the previous version. The following is an example of an update script. <?php function buddylist_update_1() { db_query("ALTER TABLE .... ..... "); } ?> These updates can also be database specific (ie: buddylist_update_1_pgsql() etc). When a module needs to be updated, it will execute each of the functions in order (exactly like one would in updates.inc currently.) I have included copies of the Trackback and Buddylist modules for testing. ------------------------------------------------------------------------ March 31, 2005 - 03:26 : FactoryJoe(a)civicspacelabs.org No idea if this is of any use, but I thought it would be cool if there were feeds for each module: http://www.speirs.org/appcasting/ Drumm calls it RSS for RSS' sake. Perhaps, but it's worth considering.

1 0

[drupal-devel] [bug] Clean up plain-text checking
by Steven 31 Mar '05

31 Mar '05

Issue status update for http://drupal.org/node/18817 Project: Drupal Version: cvs Component: base system Category: bug reports Priority: normal Assigned to: Steven Reported by: Steven Updated by: Steven Status: patch Attachment: http://drupal.org/files/issues/check.patch (91.83 KB) Back story Bart Jansens brought up some possible XSS issues recently which prompted me to reevaluate the text checking in Drupal. The big issue is "plain text": what a user typically enters in single-line boxes. When this text is put into HTML, special characters like < > and & need to be escaped. When it is put into an HTML attribute, " needs to be escaped too. Note that it is not about filtering (what check_output() does), but about handling plain-text (commonly associated with single-line editboxes). The current 'standard' in Drupal is to do escaping when outputting the titles in HTML, and keep the plain-text in the database. We should keep this as it is in line with "keep exactly what the user typed and process it later". Furthermore it is hard to undo this escaping, which is needed when communicating with other services or when exporting data. This brings up the problem: where should text be escaped? There are in fact many places where this escaping is not done yet, which means there are problems if the user enters e.g. < or & in a title. Usually these bugs are in admin areas, so not critical for security, but they should still be fixed in the spirit of XHTML validation and usability. So this means that before the call to check_plain(), the data is plain-text and no HTML can be used in it. Once the text has been run through check_plain(), it is HTML, and tags can be added. Logically, it is easy: data should be escaped as soon as it becomes HTML, but exactly deciding where data becomes HTML is tricky. Any parameter which is passed as plain-text and only checked later in the code can not contain HTML, so I've had to decide where to add missing escapings based on context and common sense. This issue also affects URLs, but there the XSS issues are more important. Plus all of this should not be confused with urlencode() which is simply used to put arbitrary data into an URL without it disrupting normal URL semantics (directories, anchors, query arguments, etc). Concrete changes drupal_specialchars() and check_form() were almost the same, except that one would escape " while the other wouldn't. This difference is negligable and it does no harm to escape too much. In the interest of cleaning up text handling, I merged these into check_plain(). check_url() had a toss at extra XSS protection in it, but it was in fact broken and wholly incorrect. I replaced it with what was probably the original intention. Many URL handling functions in fact worked with HTML-escaped URLs (using & instead of & for example). I got rid of this, and did the escaping in a more logical place (e.g. drupal_goto no longer has to reconvert the &). Node/comment titles used to be run through strip_tags(), which means you couldn't use HTML but were still forced to escape angle brackets and amps. I changed them to be consistent with other plain-text fields in Drupal. Profile checkbox items were handled inconsistently as well and were fixed. The l() function now escapes the title by default, in the interest of keeping the code cleaner. But the old behaviour is still available (and needed in some places) with an extra parameter. Text handling in Drupal after this patch Any piece of user submitted text should be run through one of the check_ functions before being put into HTML. Use check_plain() for plain-text, check_output() for rich text. Dynamic data in URLs should be urlencode()d, regardless of where that URL ends up. URLs in HTML attributes should be check_url()'d. If you are unsure if you're doing the right thing, the easiest test is to try out putting HTML tags in your module's fields where they are not intended. They should be displayed on-screen and not interpreted. Steven

5 12